[FFmpeg-devel] [PATCH] lavfi/tests: Fix 16-bit vf_blend test to avoid memory not aligned to 2 bytes

[Andrey Semashev]

Generic C implementation of vf_blend performs reads and writes of 16-bit
elements, which requires the buffers to be aligned to at least 2-byte
boundary.

Also, the change fixes source buffer overrun caused by src_offset being
added to to test handling of misaligned buffers.

Fixes: #7226
---
 tests/checkasm/vf_blend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/checkasm/vf_blend.c b/tests/checkasm/vf_blend.c
index 912f3a2c38..a7578fec39 100644
--- a/tests/checkasm/vf_blend.c
+++ b/tests/checkasm/vf_blend.c
@@ -71,7 +71,7 @@
 w = WIDTH / depth; 
\

\
 for (i = 0; i < BUF_UNITS - 1; i++) {  
\
-int src_offset = i * SIZE_PER_UNIT + i; /* Test various alignments 
*/  \
+int src_offset = i * SIZE_PER_UNIT + (BUF_UNITS - 1 - i) * depth; 
/* Test various alignments */  \
 int dst_offset = i * SIZE_PER_UNIT; /* dst must be aligned */  
\
 randomize_buffers();   
\
 call_ref(top1 + src_offset, w, bot1 + src_offset, w,   
\
-- 
2.17.0

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

Re: [FFmpeg-devel] [PATCH] lavfi/tests: Fix 16-bit vf_blend test to avoid memory not aligned to 2 bytes

[Andrey Semashev]

On 05/24/18 00:07, Andrey Semashev wrote:

Generic C implementation of vf_blend performs reads and writes of 16-bit
elements, which requires the buffers to be aligned to at least 2-byte
boundary.

Also, the change fixes source buffer overrun caused by src_offset being
added to to test handling of misaligned buffers.

Fixes: #7226


Ping? Any comments?


---
  tests/checkasm/vf_blend.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/checkasm/vf_blend.c b/tests/checkasm/vf_blend.c
index 912f3a2c38..a7578fec39 100644
--- a/tests/checkasm/vf_blend.c
+++ b/tests/checkasm/vf_blend.c
@@ -71,7 +71,7 @@
  w = WIDTH / depth;
 \

 \
  for (i = 0; i < BUF_UNITS - 1; i++) { 
 \
-int src_offset = i * SIZE_PER_UNIT + i; /* Test various alignments 
*/  \
+int src_offset = i * SIZE_PER_UNIT + (BUF_UNITS - 1 - i) * depth; 
/* Test various alignments */  \
  int dst_offset = i * SIZE_PER_UNIT; /* dst must be aligned */ 
 \
  randomize_buffers();  
 \
  call_ref(top1 + src_offset, w, bot1 + src_offset, w,  
 \



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

Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Add support for per-stream container type selection.

[Andrey Semashev]

On 11/12/18 8:20 AM, Jeyapal, Karthick wrote:


On 11/8/18 10:27 PM, Andrey Semashev wrote:

This commit restores the ability to create DASH streams with codecs
that require different containers that was lost after commit
2efdbf7367989cf9d296c25fa3d2aff8d6e25fdd. It extends the dash_segment_type
option syntax to allow to specify segment container types for individual
streams, in addition to the default container type that is applied to
all streams. The extended syntax is backward compatible with the previous
syntax.

Thanks for sending the patch. I understand your requirement completely.
But I feel that this option for mapping streams with container format is little 
confusing. Also, the relevant code is relatively big, and thus difficult to 
maintain in future.
I have a middle ground suggestion. If your goal is to achieve the earlier 
behavior broken commits, then I propose the following.
Option "dash_segment_type" could take one more option "auto" (instead of mp4 or 
webm).
When "auto" is chosen, the muxer could choose webm format for VP8, VP9, vorbis, 
opus streams and mp4 format for all other streams.
In this method the previous behavior of dashenc muxer could be restored with 
little addition to the overall code. Also it's usage will be simpler and easier 
to understand.


This solution might be ok for just restoring the previous capability, 
but I think the ability for selecting the container format by the user 
is still more useful. For example, Opus can be muxed in both mp4 
(although, with experimental flag) and webm, and it may make sense to 
some users to select mp4. (In my case, though, I wanted webm, hence the 
patch.)


Besides the parser, it doesn't add much code, and if I can improve the 
patch, please let me know how. If you absolutely don't want this 
functionality, that's ok, I'll keep this patch for my local builds and 
submit an "auto" option patch instead.



---
  doc/muxers.texi   |   8 ++-
  libavformat/dashenc.c | 161 +++---
  2 files changed, 140 insertions(+), 29 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 62f4091e31..4418b38c76 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -289,8 +289,12 @@ Set container format (mp4/webm) options using a @code{:} 
separated list of
  key=value parameters. Values containing @code{:} special characters must be
  escaped.
  
-@item dash_segment_type @var{dash_segment_type}

-Possible values:
+@item -dash_segment_type @var{dash_segment_type}
+Sets the container type for dash segment files. Syntax is " :a,b,c 
:d,e" where  is
+the container type and a, b, c, d and e are the indices of the mapped streams. 
When no indices are specified,
+the container type is set for all streams.
+
+Possible container type values:
  @item mp4
  If this flag is set, the dash segment files will be in in ISOBMFF format. 
This is the default format.
  
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c

index f8b3d106d5..626dc76413 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -84,6 +84,8 @@ typedef struct OutputStream {
  int64_t first_pts, start_pts, max_pts;
  int64_t last_dts, last_pts;
  int bit_rate;
+SegmentType segment_type;
+const char *format_name;
  
  char codec_str[100];

  int written_len;
@@ -131,8 +133,7 @@ typedef struct DASHContext {
  int64_t timeout;
  int index_correction;
  char *format_options_str;
-SegmentType segment_type;
-const char *format_name;
+const char *segment_types_str;
  } DASHContext;
  
  static struct codec_string {

@@ -188,14 +189,6 @@ static void dashenc_io_close(AVFormatContext *s, 
AVIOContext **pb, char *filenam
  }
  }
  
-static const char *get_format_str(SegmentType segment_type) {

-int i;
-for (i = 0; i < SEGMENT_TYPE_NB; i++)
-if (formats[i].segment_type == segment_type)
-return formats[i].str;
-return NULL;
-}
-
  static int check_file_extension(const char *filename, const char *extension) {
  char *dot;
  if (!filename || !extension)
@@ -375,6 +368,8 @@ static void dash_free(AVFormatContext *s)
  c->nb_as = 0;
  }
  
+av_freep(>segment_types_str);

+
  if (!c->streams)
  return;
  for (i = 0; i < s->nb_streams; i++) {
@@ -621,13 +616,13 @@ static int write_adaptation_set(AVFormatContext *s, 
AVIOContext *out, int as_ind
  if (as->media_type == AVMEDIA_TYPE_VIDEO) {
  AVStream *st = s->streams[i];
  avio_printf(out, "\t\t\tformat_name, os->codec_str, bandwidth_str, 
s->streams[i]->codecpar->width, s->streams[i]->codecpar->height);
+i, os->format_name, os->codec_str, bandwidth_str, 
s->streams[i]->codecpar->width, s->streams[i]->codecpar->height);
  if (st->avg_frame_rate.num)
 

Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Add support for per-stream container type selection.

[Andrey Semashev]

On 11/12/18 3:12 PM, Jeyapal, Karthick wrote:


On 11/12/18 5:20 PM, Andrey Semashev wrote:

On 11/12/18 8:20 AM, Jeyapal, Karthick wrote:


On 11/8/18 10:27 PM, Andrey Semashev wrote:

This commit restores the ability to create DASH streams with codecs
that require different containers that was lost after commit
2efdbf7367989cf9d296c25fa3d2aff8d6e25fdd. It extends the dash_segment_type
option syntax to allow to specify segment container types for individual
streams, in addition to the default container type that is applied to
all streams. The extended syntax is backward compatible with the previous
syntax.

Thanks for sending the patch. I understand your requirement completely.
But I feel that this option for mapping streams with container format is little 
confusing. Also, the relevant code is relatively big, and thus difficult to 
maintain in future.
I have a middle ground suggestion. If your goal is to achieve the earlier 
behavior broken commits, then I propose the following.
Option "dash_segment_type" could take one more option "auto" (instead of mp4 or 
webm).
When "auto" is chosen, the muxer could choose webm format for VP8, VP9, vorbis, 
opus streams and mp4 format for all other streams.
In this method the previous behavior of dashenc muxer could be restored with 
little addition to the overall code. Also it's usage will be simpler and easier 
to understand.


This solution might be ok for just restoring the previous capability, but I 
think the ability for selecting the container format by the user is still more 
useful. For example, Opus can be muxed in both mp4 (although, with experimental 
flag) and webm, and it may make sense to some users to select mp4. (In my case, 
though, I wanted webm, hence the patch.)

In that case they could select "dash_segment_type" as "mp4", in which case all 
streams including opus will be muxed in mp4 format. I don't see a use-case where somebody wants 
opus in mp4 and would want other streams in webm format.


Suppose you want to create a DASH stream consisting of VP8, H264, Vorbis 
and Opus substreams to cover the best compatibility with clients (which 
are mostly web browsers, but not necessarilly all of them). AFAIK, you 
cannot put all these codecs neither in mp4 nor webm. An "auto" option 
would put Opus in webm container, leaving clients not supporting webm 
not able to play audio. With explicit container selection one could put 
Opus content in mp4 or both webm and mp4 (in which case the substream 
would be duplicated).


An example of a client that is picky about container format is Safari on 
OS X High Sierra. I don't have one to test, but reportedly it supports 
Opus only in caf format, and I've read someone hacked it to play Opus in 
mp4 if disguised as AAC. I know lavf/dashenc doesn't support caf (yet) 
but it may support it in the future, so the container format selection 
would become even more relevant then.



Besides the parser, it doesn't add much code, and if I can improve the patch, please let 
me know how. If you absolutely don't want this functionality, that's ok, I'll keep this 
patch for my local builds and submit an "auto" option patch instead.

I am not absolutely against this patch. But I am just trying to find if there 
is a use-case for such an advanced option. If there is a practical requirement 
for such a use-case, then I agree that we should review and push this patch for 
sure.
But if the requirement is just theoretical at this point, then I would prefer the 
"auto" option patch. Maybe we could revisit this advanced options patch when a 
real requirement comes up.



---
   doc/muxers.texi   |   8 ++-
   libavformat/dashenc.c | 161 +++---
   2 files changed, 140 insertions(+), 29 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 62f4091e31..4418b38c76 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -289,8 +289,12 @@ Set container format (mp4/webm) options using a @code{:} 
separated list of
   key=value parameters. Values containing @code{:} special characters must be
   escaped.
   -@item dash_segment_type @var{dash_segment_type}
-Possible values:
+@item -dash_segment_type @var{dash_segment_type}
+Sets the container type for dash segment files. Syntax is " :a,b,c 
:d,e" where  is
+the container type and a, b, c, d and e are the indices of the mapped streams. 
When no indices are specified,
+the container type is set for all streams.
+
+Possible container type values:
   @item mp4
   If this flag is set, the dash segment files will be in in ISOBMFF format. 
This is the default format.
   diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index f8b3d106d5..626dc76413 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -84,6 +84,8 @@ typedef struct OutputStream {
   int64_t first_pts, start_pts, max_pts;
   int64_t last_dts, last_pts;
 

Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Add support for per-stream container type selection.

[Andrey Semashev]

On 11/12/18 3:55 PM, Andrey Semashev wrote:

On 11/12/18 3:12 PM, Jeyapal, Karthick wrote:


On 11/12/18 5:20 PM, Andrey Semashev wrote:

On 11/12/18 8:20 AM, Jeyapal, Karthick wrote:


On 11/8/18 10:27 PM, Andrey Semashev wrote:

This commit restores the ability to create DASH streams with codecs
that require different containers that was lost after commit
2efdbf7367989cf9d296c25fa3d2aff8d6e25fdd. It extends the 
dash_segment_type
option syntax to allow to specify segment container types for 
individual

streams, in addition to the default container type that is applied to
all streams. The extended syntax is backward compatible with the 
previous

syntax.

Thanks for sending the patch. I understand your requirement completely.
But I feel that this option for mapping streams with container 
format is little confusing. Also, the relevant code is relatively 
big, and thus difficult to maintain in future.
I have a middle ground suggestion. If your goal is to achieve the 
earlier behavior broken commits, then I propose the following.
Option "dash_segment_type" could take one more option "auto" 
(instead of mp4 or webm).
When "auto" is chosen, the muxer could choose webm format for VP8, 
VP9, vorbis, opus streams and mp4 format for all other streams.
In this method the previous behavior of dashenc muxer could be 
restored with little addition to the overall code. Also it's usage 
will be simpler and easier to understand.


This solution might be ok for just restoring the previous capability, 
but I think the ability for selecting the container format by the 
user is still more useful. For example, Opus can be muxed in both mp4 
(although, with experimental flag) and webm, and it may make sense to 
some users to select mp4. (In my case, though, I wanted webm, hence 
the patch.)
In that case they could select "dash_segment_type" as "mp4", in which 
case all streams including opus will be muxed in mp4 format. I don't 
see a use-case where somebody wants opus in mp4 and would want other 
streams in webm format.


Suppose you want to create a DASH stream consisting of VP8, H264, Vorbis 
and Opus substreams to cover the best compatibility with clients (which 
are mostly web browsers, but not necessarilly all of them). AFAIK, you 
cannot put all these codecs neither in mp4 nor webm. An "auto" option 
would put Opus in webm container, leaving clients not supporting webm 
not able to play audio. With explicit container selection one could put 
Opus content in mp4 or both webm and mp4 (in which case the substream 
would be duplicated).


An example of a client that is picky about container format is Safari on 
OS X High Sierra. I don't have one to test, but reportedly it supports 
Opus only in caf format, and I've read someone hacked it to play Opus in 
mp4 if disguised as AAC. I know lavf/dashenc doesn't support caf (yet) 
but it may support it in the future, so the container format selection 
would become even more relevant then.


So, what do we decide about this patch?

Besides the parser, it doesn't add much code, and if I can improve 
the patch, please let me know how. If you absolutely don't want this 
functionality, that's ok, I'll keep this patch for my local builds 
and submit an "auto" option patch instead.
I am not absolutely against this patch. But I am just trying to find 
if there is a use-case for such an advanced option. If there is a 
practical requirement for such a use-case, then I agree that we should 
review and push this patch for sure.
But if the requirement is just theoretical at this point, then I would 
prefer the "auto" option patch. Maybe we could revisit this advanced 
options patch when a real requirement comes up.



---
   doc/muxers.texi   |   8 ++-
   libavformat/dashenc.c | 161 
+++---

   2 files changed, 140 insertions(+), 29 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 62f4091e31..4418b38c76 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -289,8 +289,12 @@ Set container format (mp4/webm) options using 
a @code{:} separated list of
   key=value parameters. Values containing @code{:} special 
characters must be

   escaped.
   -@item dash_segment_type @var{dash_segment_type}
-Possible values:
+@item -dash_segment_type @var{dash_segment_type}
+Sets the container type for dash segment files. Syntax is " 
:a,b,c :d,e" where  is
+the container type and a, b, c, d and e are the indices of the 
mapped streams. When no indices are specified,

+the container type is set for all streams.
+
+Possible container type values:
   @item mp4
   If this flag is set, the dash segment files will be in in 
ISOBMFF format. This is the default format.

   diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index f8b3d106d5..626dc76413 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -84,6 +84,8 @@ typedef struct Outp

Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Add support for per-stream container type selection.

[Andrey Semashev]

On 11/14/18 4:52 PM, Jeyapal, Karthick wrote:


On 11/14/18 1:41 PM, Andrey Semashev wrote:

On 11/12/18 3:55 PM, Andrey Semashev wrote:


Suppose you want to create a DASH stream consisting of VP8, H264, Vorbis
and Opus substreams to cover the best compatibility with clients (which
are mostly web browsers, but not necessarilly all of them). AFAIK, you
cannot put all these codecs neither in mp4 nor webm. An "auto" option
would put Opus in webm container, leaving clients not supporting webm
not able to play audio. With explicit container selection one could put
Opus content in mp4 or both webm and mp4 (in which case the substream
would be duplicated).

An example of a client that is picky about container format is Safari on
OS X High Sierra. I don't have one to test, but reportedly it supports
Opus only in caf format, and I've read someone hacked it to play Opus in
mp4 if disguised as AAC. I know lavf/dashenc doesn't support caf (yet)
but it may support it in the future, so the container format selection
would become even more relevant then.


So, what do we decide about this patch?

I understand your point. But all your examples are still theoretical(say caf is 
not supported in dashenc. And opus example doesn't mention any sample clients 
for that use-case).
My view is that, if we can find one practical example where this advanced patch 
will be of immediate use(which cannot be done by auto option), then let us 
review and push this patch.
If we can't find any immediate practical use-case, which could be tested and 
verified in popular clients then let's go for auto option.


Ok, thanks for your comments.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] lavf/dashenc: Add support for per-stream container type selection.

[Andrey Semashev]

This commit restores the ability to create DASH streams with codecs
that require different containers that was lost after commit
2efdbf7367989cf9d296c25fa3d2aff8d6e25fdd. It extends the dash_segment_type
option syntax to allow to specify segment container types for individual
streams, in addition to the default container type that is applied to
all streams. The extended syntax is backward compatible with the previous
syntax.
---
 doc/muxers.texi   |   8 ++-
 libavformat/dashenc.c | 161 +++---
 2 files changed, 140 insertions(+), 29 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 62f4091e31..4418b38c76 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -289,8 +289,12 @@ Set container format (mp4/webm) options using a @code{:} 
separated list of
 key=value parameters. Values containing @code{:} special characters must be
 escaped.
 
-@item dash_segment_type @var{dash_segment_type}
-Possible values:
+@item -dash_segment_type @var{dash_segment_type}
+Sets the container type for dash segment files. Syntax is " :a,b,c 
:d,e" where  is
+the container type and a, b, c, d and e are the indices of the mapped streams. 
When no indices are specified,
+the container type is set for all streams.
+
+Possible container type values:
 @item mp4
 If this flag is set, the dash segment files will be in in ISOBMFF format. This 
is the default format.
 
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index f8b3d106d5..626dc76413 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -84,6 +84,8 @@ typedef struct OutputStream {
 int64_t first_pts, start_pts, max_pts;
 int64_t last_dts, last_pts;
 int bit_rate;
+SegmentType segment_type;
+const char *format_name;
 
 char codec_str[100];
 int written_len;
@@ -131,8 +133,7 @@ typedef struct DASHContext {
 int64_t timeout;
 int index_correction;
 char *format_options_str;
-SegmentType segment_type;
-const char *format_name;
+const char *segment_types_str;
 } DASHContext;
 
 static struct codec_string {
@@ -188,14 +189,6 @@ static void dashenc_io_close(AVFormatContext *s, 
AVIOContext **pb, char *filenam
 }
 }
 
-static const char *get_format_str(SegmentType segment_type) {
-int i;
-for (i = 0; i < SEGMENT_TYPE_NB; i++)
-if (formats[i].segment_type == segment_type)
-return formats[i].str;
-return NULL;
-}
-
 static int check_file_extension(const char *filename, const char *extension) {
 char *dot;
 if (!filename || !extension)
@@ -375,6 +368,8 @@ static void dash_free(AVFormatContext *s)
 c->nb_as = 0;
 }
 
+av_freep(>segment_types_str);
+
 if (!c->streams)
 return;
 for (i = 0; i < s->nb_streams; i++) {
@@ -621,13 +616,13 @@ static int write_adaptation_set(AVFormatContext *s, 
AVIOContext *out, int as_ind
 if (as->media_type == AVMEDIA_TYPE_VIDEO) {
 AVStream *st = s->streams[i];
 avio_printf(out, "\t\t\tformat_name, os->codec_str, bandwidth_str, 
s->streams[i]->codecpar->width, s->streams[i]->codecpar->height);
+i, os->format_name, os->codec_str, bandwidth_str, 
s->streams[i]->codecpar->width, s->streams[i]->codecpar->height);
 if (st->avg_frame_rate.num)
 avio_printf(out, " frameRate=\"%d/%d\"", 
st->avg_frame_rate.num, st->avg_frame_rate.den);
 avio_printf(out, ">\n");
 } else {
 avio_printf(out, "\t\t\t\n",
-i, c->format_name, os->codec_str, bandwidth_str, 
s->streams[i]->codecpar->sample_rate);
+i, os->format_name, os->codec_str, bandwidth_str, 
s->streams[i]->codecpar->sample_rate);
 avio_printf(out, "\t\t\t\t\n",
 s->streams[i]->codecpar->channels);
 }
@@ -773,6 +768,120 @@ end:
 return 0;
 }
 
+static inline void set_all_segment_types(AVFormatContext *s, int format_idx)
+{
+DASHContext *c = s->priv_data;
+for (int i = 0; i < s->nb_streams; ++i) {
+OutputStream *os = >streams[i];
+os->segment_type = formats[format_idx].segment_type;
+os->format_name = formats[format_idx].str;
+}
+}
+
+static int parse_segment_types(AVFormatContext *s)
+{
+DASHContext *c = s->priv_data;
+const char *p = c->segment_types_str;
+enum { type_expected, type_parsed, parsing_streams } state;
+int i, format_idx;
+
+// Set the default container type in case if some streams are not 
mentioned in the string
+set_all_segment_types(s, 0);
+
+if (!p)
+return 0;
+
+// Parse per-stream container types: mp4 webm:0,1,2 and so on
+state = type_expected;
+format_idx = 0;
+while (*p) {
+if (*p == ' ') {
+if (state == type_parsed)
+set_all_segment_types(s, format_idx);
+
+state = type_expected;
+++p;
+continue;
+}
+
+if (state == type_expected) {
+   

Re: [FFmpeg-devel] [PATCH v3 1/2] lavf: Add general API for IO buffer synchronization.

[Andrey Semashev]

On 12/21/18 2:48 AM, Michael Niedermayer wrote:

On Thu, Dec 20, 2018 at 11:00:32AM +0100, Nicolas George wrote:

Andrey Semashev (2018-12-10):

This commit adds a new set of functions to avio and url subsystems, which
allow users to invoke IO buffer synchronization with the underlying media.
The most obvious target for this extension if the filesystem streams. Invoking
IO synchronization allows user applications to ensure that all written content
has reached the filesystem on the media and can be observed by other processes.

The public API for this is avio_sync() function, which can be called on
AVIOContext. The internal, lower layer API is ffurl_sync(), which works
directly on the underlying URLContext. URLContext backends can add support for
this new API by setting the sync handler to the new url_sync member of
URLProtocol. When no handler is set then the sync operation is a no-op,
the result code is AVERROR(ENOSYS).
---
  libavformat/avio.c|  7 +++
  libavformat/avio.h| 20 
  libavformat/aviobuf.c | 17 +
  libavformat/url.h | 12 
  4 files changed, 56 insertions(+)


I have no more objections to the patch as it is, thanks. But I have no
opinion in favor of it either; it is useful in principle, but I do not
know if it is worth the extra maintenance for a use case like this
project. I will leave the judgement to others.


IMHO, it looks useful and extra maintaince needed seems not major


So, will this patchset be merged then?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3 1/2] lavf: Add general API for IO buffer synchronization.

[Andrey Semashev]

Ping?

On December 10, 2018 3:05:54 PM Andrey Semashev  
wrote:



This commit adds a new set of functions to avio and url subsystems, which
allow users to invoke IO buffer synchronization with the underlying media.
The most obvious target for this extension if the filesystem streams. Invoking
IO synchronization allows user applications to ensure that all written content
has reached the filesystem on the media and can be observed by other processes.

The public API for this is avio_sync() function, which can be called on
AVIOContext. The internal, lower layer API is ffurl_sync(), which works
directly on the underlying URLContext. URLContext backends can add support for
this new API by setting the sync handler to the new url_sync member of
URLProtocol. When no handler is set then the sync operation is a no-op,
the result code is AVERROR(ENOSYS).
---
libavformat/avio.c|  7 +++
libavformat/avio.h| 20 
libavformat/aviobuf.c | 17 +
libavformat/url.h | 12 
4 files changed, 56 insertions(+)

diff --git a/libavformat/avio.c b/libavformat/avio.c
index 663789ec02..5754a7c20d 100644
--- a/libavformat/avio.c
+++ b/libavformat/avio.c
@@ -623,6 +623,13 @@ int64_t ffurl_size(URLContext *h)
return size;
}

+int ffurl_sync(URLContext *h)
+{
+if (h->prot->url_sync)
+return h->prot->url_sync(h);
+return AVERROR(ENOSYS);
+}
+
int ffurl_get_file_handle(URLContext *h)
{
if (!h || !h->prot || !h->prot->url_get_file_handle)
diff --git a/libavformat/avio.h b/libavformat/avio.h
index 75912ce6be..9e2ef14e60 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -349,6 +349,14 @@ typedef struct AVIOContext {
 * Try to buffer at least this amount of data before flushing it
 */
int min_packet_size;
+
+/**
+ * A callback for synchronizing buffers with the media state.
+ *
+ * @return 0 on success, AVERROR(ENOSYS) if the operation is not supported
+ * and ignored, or other AVERROR < 0 on error.
+ */
+int (*sync)(void *opaque);
} AVIOContext;

/**
@@ -586,6 +594,18 @@ int avio_printf(AVIOContext *s, const char *fmt, ...) 
av_printf_format(2, 3);

 */
void avio_flush(AVIOContext *s);

+/**
+ * Flush internal buffers and ensure the synchronized state of the
+ * resource associated with the context s. This call may be expensive.
+ * Not all resources support syncing, this operation is a no-op
+ * if sync is not supported or needed.
+ * This function can only be used if s was opened by avio_open().
+ *
+ * @return 0 on success, AVERROR(ENOSYS) if the operation is not supported
+ * and ignored, or other AVERROR < 0 on error.
+ */
+int avio_sync(AVIOContext *s);
+
/**
 * Read size bytes from AVIOContext into buf.
 * @return number of bytes read or AVERROR
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 5a33f82950..c1c9334719 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -243,6 +243,14 @@ void avio_flush(AVIOContext *s)
avio_seek(s, seekback, SEEK_CUR);
}

+int avio_sync(AVIOContext *s)
+{
+avio_flush(s);
+if (s->sync)
+return s->sync(s->opaque);
+return AVERROR(ENOSYS);
+}
+
int64_t avio_seek(AVIOContext *s, int64_t offset, int whence)
{
int64_t offset1;
@@ -978,6 +986,12 @@ static int64_t io_read_seek(void *opaque, int 
stream_index, int64_t timestamp, i
return internal->h->prot->url_read_seek(internal->h, stream_index, 
timestamp, flags);

}

+static int io_sync(void *opaque)
+{
+AVIOInternal *internal = opaque;
+return ffurl_sync(internal->h);
+}
+
int ffio_fdopen(AVIOContext **s, URLContext *h)
{
AVIOInternal *internal = NULL;
@@ -1026,6 +1040,9 @@ int ffio_fdopen(AVIOContext **s, URLContext *h)

if (h->prot->url_read_seek)
(*s)->seekable |= AVIO_SEEKABLE_TIME;
+
+if (h->prot->url_sync)
+(*s)->sync = io_sync;
}
(*s)->short_seek_get = io_short_seek;
(*s)->av_class = _avio_class;
diff --git a/libavformat/url.h b/libavformat/url.h
index 4750bfff82..5b8cd22e5a 100644
--- a/libavformat/url.h
+++ b/libavformat/url.h
@@ -97,6 +97,7 @@ typedef struct URLProtocol {
int (*url_delete)(URLContext *h);
int (*url_move)(URLContext *h_src, URLContext *h_dst);
const char *default_whitelist;
+int (*url_sync)(URLContext *h);
} URLProtocol;

/**
@@ -228,6 +229,17 @@ int64_t ffurl_seek(URLContext *h, int64_t pos, int 
whence);

int ffurl_closep(URLContext **h);
int ffurl_close(URLContext *h);

+/**
+ * Flush any buffered data and synchronize the resource accessed
+ * by the URLContext h. This call may be expensive.
+ * Not all types of resources support syncing, the call is a no-op
+ * if sync is not supported or needed.
+ *
+ * @return 0 on success, AVERROR(ENOSYS) if the operation is not supported
+ * and ignored, or other AVERROR < 0 on error.
+ */
+int ffurl_sync(URLCo

[FFmpeg-devel] [PATCH] lavf/dashenc: Write media trailers when DASH trailer is written.

[Andrey Semashev]

This commit ensures that all (potentially, long) filesystem activity is
performed when the user calls av_write_trailer on the DASH libavformat
context, not when freeing the context. Also, this defers media segment
deletion until after the media trailers are written.
---
 libavformat/dashenc.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 6ce70e0076..e1c959dc89 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -424,8 +424,6 @@ static void dash_free(AVFormatContext *s)
 return;
 for (i = 0; i < s->nb_streams; i++) {
 OutputStream *os = >streams[i];
-if (os->ctx && os->ctx_inited)
-av_write_trailer(os->ctx);
 if (os->ctx && os->ctx->pb)
 ffio_free_dyn_buf(>ctx->pb);
 ff_format_io_close(s, >out);
@@ -1420,13 +1418,11 @@ static int dash_flush(AVFormatContext *s, int final, 
int stream)
 os->pos += range_length;
 }
 
-if (c->window_size || (final && c->remove_at_exit)) {
+if (c->window_size) {
 for (i = 0; i < s->nb_streams; i++) {
 OutputStream *os = >streams[i];
 int j;
 int remove = os->nb_segments - c->window_size - 
c->extra_window_size;
-if (final && c->remove_at_exit)
-remove = os->nb_segments;
 if (remove > 0) {
 for (j = 0; j < remove; j++) {
 char filename[1024];
@@ -1599,11 +1595,24 @@ static int dash_write_trailer(AVFormatContext *s)
 }
 dash_flush(s, 1, -1);
 
+for (int i = 0; i < s->nb_streams; ++i) {
+OutputStream *os = >streams[i];
+if (os->ctx && os->ctx_inited) {
+av_write_trailer(os->ctx);
+}
+}
+
 if (c->remove_at_exit) {
 char filename[1024];
 int i;
 for (i = 0; i < s->nb_streams; i++) {
 OutputStream *os = >streams[i];
+for (int j = 0; j < os->nb_segments; ++j) {
+snprintf(filename, sizeof(filename), "%s%s", c->dirname, 
os->segments[j]->file);
+dashenc_delete_file(s, filename);
+av_free(os->segments[j]);
+}
+os->nb_segments = 0;
 snprintf(filename, sizeof(filename), "%s%s", c->dirname, 
os->initfile);
 dashenc_delete_file(s, filename);
 }
-- 
2.19.1

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

[FFmpeg-devel] [PATCH] lavf/dashenc: Fix file URI handling when deleting files.

[Andrey Semashev]

The URI used to open the output streams may be an actual URI with "file" scheme,
according to https://tools.ietf.org/html/rfc8089. This commit makes file
deletion routine recognize file URIs and extract the actual filesystem path
from it.

It also fixes strerror use, which may not be thread-safe.
---
 libavformat/dashenc.c | 29 +++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 6ce70e0076..e59fa0944e 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -25,6 +25,7 @@
 #include 
 #endif
 
+#include "libavutil/error.h"
 #include "libavutil/avassert.h"
 #include "libavutil/avutil.h"
 #include "libavutil/avstring.h"
@@ -1326,8 +1327,32 @@ static void dashenc_delete_file(AVFormatContext *s, char 
*filename) {
 
 av_dict_free(_opts);
 ff_format_io_close(s, );
-} else if (unlink(filename) < 0) {
-av_log(s, AV_LOG_ERROR, "failed to delete %s: %s\n", filename, 
strerror(errno));
+} else {
+const char* path = filename;
+// Check if the filename is a file URI. 
https://tools.ietf.org/html/rfc8089#section-2
+if (av_strncasecmp(path, "file:", sizeof("file:") - 1) == 0) {
+path += sizeof("file:") - 1;
+if (path[0] == '/' && path[1] == '/') {
+// The URI may have an authority part. Check that the 
authority does not contain
+// a host name. We cannot access filesystem on a different 
host.
+path += 2;
+if (path[0] != '/') {
+if (strncmp(path, "localhost", sizeof("localhost") - 1) == 
0) {
+path += sizeof("localhost") - 1;
+} else {
+av_log(s, AV_LOG_ERROR, "cannot delete file on a 
remote host: %s\n", filename);
+return;
+}
+}
+}
+}
+
+if (unlink(path) < 0) {
+int err = AVERROR(errno);
+char errbuf[128];
+av_strerror(err, errbuf, sizeof(errbuf));
+av_log(s, (err == AVERROR(ENOENT) ? AV_LOG_WARNING : 
AV_LOG_ERROR), "failed to delete %s: %s\n", path, errbuf);
+}
 }
 }
 
-- 
2.19.1

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

[FFmpeg-devel] [PATCH] lavf/dashenc: Remove global_sidx from movenc params for live streaming.

[Andrey Semashev]

The global_sidx flag causes errors like the following in movenc when media
segment removal is enabled via windos_size or remove_at_exit:

Non-consecutive fragments, writing incorrect sidx
Unable to re-open  output file for the second pass (faststart)
---
 libavformat/dashenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 6ce70e0076..cb49641b4e 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -1141,7 +1141,7 @@ static int dash_init(AVFormatContext *s)
 
 if (os->segment_type == SEGMENT_TYPE_MP4) {
 if (c->streaming)
-av_dict_set(, "movflags", 
"frag_every_frame+dash+delay_moov+global_sidx", 0);
+av_dict_set(, "movflags", 
"frag_every_frame+dash+delay_moov", 0);
 else
 av_dict_set(, "movflags", "frag_custom+dash+delay_moov", 
0);
 } else {
-- 
2.19.1

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

[FFmpeg-devel] [PATCH 2/2] lavf/dashenc: Delete HLS manifests on trailer writing if remove_at_exit is set.

[Andrey Semashev]

This fixes HLS manifests being left behind if remove_at_exit is set.
---
 libavformat/dashenc.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index a7d8c4e237..af3f0ee167 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -1619,6 +1619,18 @@ static int dash_write_trailer(AVFormatContext *s)
 dashenc_delete_file(s, filename);
 }
 dashenc_delete_file(s, s->url);
+
+if (c->hls_playlist && c->master_playlist_created) {
+for (i = 0; i < s->nb_streams; i++) {
+OutputStream *os = >streams[i];
+if (os->segment_type == SEGMENT_TYPE_MP4) {
+get_hls_playlist_name(filename, sizeof(filename), 
c->dirname, i);
+dashenc_delete_file(s, filename);
+}
+}
+snprintf(filename, sizeof(filename), "%smaster.m3u8", c->dirname);
+dashenc_delete_file(s, filename);
+}
 }
 
 return 0;
-- 
2.19.1

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

[FFmpeg-devel] [PATCH 1/2] lavf/dashenc: Don't put non-mp4 streams in HLS manifests.

[Andrey Semashev]

The only native HLS implementation in the wild (Safari browser) doesn't
support WebM. And at least some MSE-based players (e.g. shaka-player)
cannot handle WebM media segments when playing HLS. So just skip non-mp4
streams from HLS manifests. Note that such streams will still be described
by the DASH manifest and therefore consumed by players supporting DASH.
---
 libavformat/dashenc.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 6ce70e0076..a7d8c4e237 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -225,6 +225,7 @@ static inline SegmentType select_segment_type(SegmentType 
segment_type, enum AVC
 static int init_segment_types(AVFormatContext *s)
 {
 DASHContext *c = s->priv_data;
+int has_mp4_streams = 0;
 for (int i = 0; i < s->nb_streams; ++i) {
 OutputStream *os = >streams[i];
 SegmentType segment_type = select_segment_type(
@@ -235,6 +236,12 @@ static int init_segment_types(AVFormatContext *s)
 av_log(s, AV_LOG_ERROR, "Could not select DASH segment type for 
stream %d\n", i);
 return AVERROR_MUXER_NOT_FOUND;
 }
+has_mp4_streams |= segment_type == SEGMENT_TYPE_MP4;
+}
+
+if (c->hls_playlist && !has_mp4_streams) {
+ av_log(s, AV_LOG_WARNING, "No mp4 streams, disabling HLS manifest 
generation\n");
+ c->hls_playlist = 0;
 }
 
 return 0;
@@ -510,7 +517,7 @@ static void output_segment_list(OutputStream *os, 
AVIOContext *out, AVFormatCont
 }
 avio_printf(out, "\t\t\t\t\n");
 }
-if (c->hls_playlist && start_index < os->nb_segments)
+if (c->hls_playlist && start_index < os->nb_segments && os->segment_type 
== SEGMENT_TYPE_MP4)
 {
 int timescale = os->ctx->streams[0]->time_base.den;
 char temp_filename_hls[1024];
@@ -944,6 +951,8 @@ static int write_manifest(AVFormatContext *s, int final)
 OutputStream *os = >streams[i];
 if (st->codecpar->codec_type != AVMEDIA_TYPE_AUDIO)
 continue;
+if (os->segment_type != SEGMENT_TYPE_MP4)
+continue;
 get_hls_playlist_name(playlist_file, sizeof(playlist_file), NULL, 
i);
 ff_hls_write_audio_rendition(c->m3u8_out, (char *)audio_group,
  playlist_file, i, is_default);
@@ -967,6 +976,8 @@ static int write_manifest(AVFormatContext *s, int final)
 int stream_bitrate = st->codecpar->bit_rate + os->muxer_overhead;
 if (st->codecpar->codec_type != AVMEDIA_TYPE_VIDEO)
 continue;
+if (os->segment_type != SEGMENT_TYPE_MP4)
+continue;
 av_strlcpy(codec_str, os->codec_str, sizeof(codec_str));
 if (max_audio_bitrate) {
 agroup = (char *)audio_group;
-- 
2.19.1

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

Re: [FFmpeg-devel] [PATCH 1/2] lavf/file: Add proper support for authority in file URIs.

[Andrey Semashev]

On 11/29/18 9:47 PM, Nicolas George wrote:

Andrey Semashev (2018-11-29):

It does. avformat_open_input, avio_open and ffurl_open[_whitelist] docs all
say it's an URL and they don't perform any conversion. So the file backend
should be prepared to receive a URL, with a scheme and authority.


So either the docs are slightly wrong or the code is. Do you have an
argument to decide it is one rather than the other?

I do:


It will probably currently fail because of the escape sequence.


Exactly. Since escaping, a very basic feature of URIs, is not handled at
all, it is a clear indication that the paths are NOT meant to be
considered URIs. The documentation was added much later, and made the
same mistake you are doing now; same goes for a few private function
names.


I condider the lack of support for escape sequences a bug, which is 
probably a rudiment of the past, when ffmpeg was primarily targeted for 
working with local files. The fact that all these functions also accept 
raw filesystem paths instead of URIs is also there for the same reason, 
only with additional benefit of convenience. Nowdays, there is one 
common interface for interacting with ffmpeg, and this interface is URIs 
(or raw local paths). There is no third pseudo-URI option, AFAICT. So, 
in my humble opinion the docs are correct, it is the implementation that 
needs to catch up.



Escape sequences, if needed, can be fixed separately.


That would break a lot of working applications and is therefore not a
good idea.


If an application passes a URI and expects that it is not interpreted as 
such is already broken. I could make a patch adding support for escape 
sequences as well, but it seems you would not accept it. Am I correct?

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

[FFmpeg-devel] [PATCH v2] lavf/dashenc: Write media trailers when DASH trailer is written.

[Andrey Semashev]

This commit ensures that all (potentially, long) filesystem activity is
performed when the user calls av_write_trailer on the DASH libavformat
context, not when freeing the context. Also, this defers media segment
deletion until after the media trailers are written.
---
 libavformat/dashenc.c | 82 ++-
 1 file changed, 58 insertions(+), 24 deletions(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 6ce70e0076..ecfd84a32c 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -424,8 +424,6 @@ static void dash_free(AVFormatContext *s)
 return;
 for (i = 0; i < s->nb_streams; i++) {
 OutputStream *os = >streams[i];
-if (os->ctx && os->ctx_inited)
-av_write_trailer(os->ctx);
 if (os->ctx && os->ctx->pb)
 ffio_free_dyn_buf(>ctx->pb);
 ff_format_io_close(s, >out);
@@ -1331,6 +1329,47 @@ static void dashenc_delete_file(AVFormatContext *s, char 
*filename) {
 }
 }
 
+static int dashenc_delete_segment_file(AVFormatContext *s, const char* file)
+{
+DASHContext *c = s->priv_data;
+size_t dirname_len, file_len;
+char filename[1024];
+
+dirname_len = strlen(c->dirname);
+if (dirname_len >= sizeof(filename)) {
+av_log(s, AV_LOG_WARNING, "Cannot delete segments as the directory 
path is too long: %"PRIu64" characters: %s\n",
+(uint64_t)dirname_len, c->dirname);
+return AVERROR(ENAMETOOLONG);
+}
+
+memcpy(filename, c->dirname, dirname_len);
+
+file_len = strlen(file);
+if ((dirname_len + file_len) >= sizeof(filename)) {
+av_log(s, AV_LOG_WARNING, "Cannot delete segments as the path is too 
long: %"PRIu64" characters: %s%s\n",
+(uint64_t)(dirname_len + file_len), c->dirname, file);
+return AVERROR(ENAMETOOLONG);
+}
+
+memcpy(filename + dirname_len, file, file_len + 1); // include the 
terminating zero
+dashenc_delete_file(s, filename);
+
+return 0;
+}
+
+static inline void dashenc_delete_media_segments(AVFormatContext *s, 
OutputStream *os, int remove_count)
+{
+for (int i = 0; i < remove_count; ++i) {
+dashenc_delete_segment_file(s, os->segments[i]->file);
+
+// Delete the segment regardless of whether the file was successfully 
deleted
+av_free(os->segments[i]);
+}
+
+os->nb_segments -= remove_count;
+memmove(os->segments, os->segments + remove_count, os->nb_segments * 
sizeof(*os->segments));
+}
+
 static int dash_flush(AVFormatContext *s, int final, int stream)
 {
 DASHContext *c = s->priv_data;
@@ -1420,23 +1459,12 @@ static int dash_flush(AVFormatContext *s, int final, 
int stream)
 os->pos += range_length;
 }
 
-if (c->window_size || (final && c->remove_at_exit)) {
+if (c->window_size) {
 for (i = 0; i < s->nb_streams; i++) {
 OutputStream *os = >streams[i];
-int j;
-int remove = os->nb_segments - c->window_size - 
c->extra_window_size;
-if (final && c->remove_at_exit)
-remove = os->nb_segments;
-if (remove > 0) {
-for (j = 0; j < remove; j++) {
-char filename[1024];
-snprintf(filename, sizeof(filename), "%s%s", c->dirname, 
os->segments[j]->file);
-dashenc_delete_file(s, filename);
-av_free(os->segments[j]);
-}
-os->nb_segments -= remove;
-memmove(os->segments, os->segments + remove, os->nb_segments * 
sizeof(*os->segments));
-}
+int remove_count = os->nb_segments - c->window_size - 
c->extra_window_size;
+if (remove_count > 0)
+dashenc_delete_media_segments(s, os, remove_count);
 }
 }
 
@@ -1584,6 +1612,7 @@ static int dash_write_packet(AVFormatContext *s, AVPacket 
*pkt)
 static int dash_write_trailer(AVFormatContext *s)
 {
 DASHContext *c = s->priv_data;
+int i;
 
 if (s->nb_streams > 0) {
 OutputStream *os = >streams[0];
@@ -1599,14 +1628,19 @@ static int dash_write_trailer(AVFormatContext *s)
 }
 dash_flush(s, 1, -1);
 
-if (c->remove_at_exit) {
-char filename[1024];
-int i;
-for (i = 0; i < s->nb_streams; i++) {
-OutputStream *os = >streams[i];
-snprintf(filename, sizeof(filename), "%s%s", c->dirname, 
os->initfile);
-dashenc_delete_file(s, filename);
+for (i = 0; i < s->nb_streams; ++i) {
+OutputStream *os = >streams[i];
+if (os->ctx && os->ctx_inited) {
+av_write_trailer(os->ctx);
+}
+
+if (c->remove_at_exit) {
+dashenc_delete_media_segments(s, os, os->nb_segments);
+dashenc_delete_segment_file(s, os->initfile);
 }
+}
+
+if (c->remove_at_exit) {
 dashenc_delete_file(s, s->url);
 }
 
-- 
2.19.1

Re: [FFmpeg-devel] [PATCH 1/2] lavf/file: Add proper support for authority in file URIs.

[Andrey Semashev]

On 11/29/18 9:24 PM, Nicolas George wrote:

Andrey Semashev (2018-11-29):

Previously, URIs with authority field were incorrectly interpreted as if
the authority was part of the path.


The "file:" prefix does not indicate a file:// URI but a path for the
file: protocol of FFmpeg.


It does. avformat_open_input, avio_open and ffurl_open[_whitelist] docs 
all say it's an URL and they don't perform any conversion. So the file 
backend should be prepared to receive a URL, with a scheme and authority.



You can check by yourself that they are not URIs by trying to get FFmpeg
to open file:///dev/nul%6c for example.


It will probably currently fail because of the escape sequence. But even 
if it weren't for this reason, it would still be interpreting the URI 
the wrong way because of the authority part, which is what this patch fixes.


Escape sequences, if needed, can be fixed separately.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] lavf/file: Add proper support for authority in file URIs.

[Andrey Semashev]

On 11/29/18 10:16 PM, Nicolas George wrote:

Andrey Semashev (2018-11-29):

 Nowdays, there is one common interface
for interacting with ffmpeg, and this interface is URIs (or raw local
paths). There is no third pseudo-URI option, AFAICT. So, in my humble
opinion the docs are correct, it is the implementation that needs to catch
up.


You are wrong. There is one common interface: that is pseudi-URI. URI is
not an option.


If an application passes a URI and expects that it is not interpreted as
such is already broken.


And it always was. Breaking something that works is worse than having
something that never worked still not work.


  I could make a patch adding support for escape
sequences as well, but it seems you would not accept it. Am I correct?


As is, "fixing" the file: protocol paths to be treated as URIs would be
an API break, it is not acceptable.

You can propose patches to make FFmpeg support real URIs instead / in
addition to its old pseudo-URI syntax, but you would need to design with
API compatibility in mind.


Ok, thanks for your comments.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v2 2/2] lavf/file: Add support for file syncing.

[Andrey Semashev]

This commit adds support for IO synchronization API to the file backend.
---
 libavformat/file.c   | 11 +++
 libavformat/os_support.h |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/libavformat/file.c b/libavformat/file.c
index 1d321c4205..58fd55b928 100644
--- a/libavformat/file.c
+++ b/libavformat/file.c
@@ -254,6 +254,16 @@ static int64_t file_seek(URLContext *h, int64_t pos, int 
whence)
 return ret < 0 ? AVERROR(errno) : ret;
 }
 
+static int file_sync(URLContext *h)
+{
+if (h->flags & AVIO_FLAG_WRITE) {
+FileContext *c = h->priv_data;
+if (fsync(c->fd) < 0)
+return AVERROR(errno);
+}
+return 0;
+}
+
 static int file_close(URLContext *h)
 {
 FileContext *c = h->priv_data;
@@ -353,6 +363,7 @@ const URLProtocol ff_file_protocol = {
 .url_close   = file_close,
 .url_get_file_handle = file_get_handle,
 .url_check   = file_check,
+.url_sync= file_sync,
 .url_delete  = file_delete,
 .url_move= file_move,
 .priv_data_size  = sizeof(FileContext),
diff --git a/libavformat/os_support.h b/libavformat/os_support.h
index 7a56dc9a7c..1864763cb1 100644
--- a/libavformat/os_support.h
+++ b/libavformat/os_support.h
@@ -93,6 +93,8 @@ static inline int is_dos_path(const char *path)
 #ifndef S_IWUSR
 #define S_IWUSR S_IWRITE
 #endif
+
+#define fsync(fd) _commit((fd))
 #endif
 
 #if CONFIG_NETWORK
-- 
2.19.1

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

[FFmpeg-devel] [PATCH v2 1/2] lavf: Add general API for IO buffer synchronization.

[Andrey Semashev]

This commit adds a new set of functions to avio and url subsystems, which
allow users to invoke IO buffer synchronization with the underlying media.
The most obvious target for this extension if the filesystem streams. Invoking
IO synchronization allows user applications to ensure that all written content
has reached the filesystem on the media and can be observed by other processes.

The public API for this is avio_sync() function, which can be called on
AVIOContext. The internal, lower layer API is ffurl_sync(), which works
directly on the underlying URLContext. URLContext backends can add support for
this new API by setting the sync handler to the new url_sync member of
URLProtocol. When no handler is set then the sync operation is a no-op.
Users can distinguish this case from the successful completion by the result
code AVIO_SYNC_NOT_SUPPORTED, which is also considered a successful result
(i.e. >0).
---
 libavformat/avio.c|  7 +++
 libavformat/avio.h| 33 +
 libavformat/aviobuf.c | 17 +
 libavformat/url.h | 13 +
 4 files changed, 70 insertions(+)

diff --git a/libavformat/avio.c b/libavformat/avio.c
index 663789ec02..662d4ed971 100644
--- a/libavformat/avio.c
+++ b/libavformat/avio.c
@@ -623,6 +623,13 @@ int64_t ffurl_size(URLContext *h)
 return size;
 }
 
+int ffurl_sync(URLContext *h)
+{
+if (h->prot->url_sync)
+return h->prot->url_sync(h);
+return AVIO_SYNC_NOT_SUPPORTED;
+}
+
 int ffurl_get_file_handle(URLContext *h)
 {
 if (!h || !h->prot || !h->prot->url_get_file_handle)
diff --git a/libavformat/avio.h b/libavformat/avio.h
index 75912ce6be..403ee0fc7b 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -349,6 +349,15 @@ typedef struct AVIOContext {
  * Try to buffer at least this amount of data before flushing it
  */
 int min_packet_size;
+
+/**
+ * A callback for synchronizing buffers with the media state.
+ *
+ * @return AVIO_SYNC_SUCCESS on success, AVIO_SYNC_NOT_SUPPORTED if
+ * the operation is not supported and ignored, an AVERROR < 0
+ * on error.
+ */
+int (*sync)(void *opaque);
 } AVIOContext;
 
 /**
@@ -586,6 +595,30 @@ int avio_printf(AVIOContext *s, const char *fmt, ...) 
av_printf_format(2, 3);
  */
 void avio_flush(AVIOContext *s);
 
+/**
+ * Indicates that the sync operation has been carried out successfully
+ */
+#define AVIO_SYNC_SUCCESS 0
+
+/**
+ * Indicates that the sync operation is not supported by the underlying
+ * resource and has been ignored
+ */
+#define AVIO_SYNC_NOT_SUPPORTED 1
+
+/**
+ * Flush internal buffers and ensure the synchronized state of the
+ * resource associated with the context s. This call may be expensive.
+ * Not all resources support syncing, this operation is a no-op
+ * if sync is not supported or needed.
+ * This function can only be used if s was opened by avio_open().
+ *
+ * @return AVIO_SYNC_SUCCESS on success, AVIO_SYNC_NOT_SUPPORTED if
+ * the operation is a not supported and ignored, an AVERROR < 0
+ * on error.
+ */
+int avio_sync(AVIOContext *s);
+
 /**
  * Read size bytes from AVIOContext into buf.
  * @return number of bytes read or AVERROR
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 5a33f82950..c2aca7c6af 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -243,6 +243,14 @@ void avio_flush(AVIOContext *s)
 avio_seek(s, seekback, SEEK_CUR);
 }
 
+int avio_sync(AVIOContext *s)
+{
+avio_flush(s);
+if (s->sync)
+return s->sync(s->opaque);
+return AVIO_SYNC_NOT_SUPPORTED;
+}
+
 int64_t avio_seek(AVIOContext *s, int64_t offset, int whence)
 {
 int64_t offset1;
@@ -978,6 +986,12 @@ static int64_t io_read_seek(void *opaque, int 
stream_index, int64_t timestamp, i
 return internal->h->prot->url_read_seek(internal->h, stream_index, 
timestamp, flags);
 }
 
+static int io_sync(void *opaque)
+{
+AVIOInternal *internal = opaque;
+return ffurl_sync(internal->h);
+}
+
 int ffio_fdopen(AVIOContext **s, URLContext *h)
 {
 AVIOInternal *internal = NULL;
@@ -1026,6 +1040,9 @@ int ffio_fdopen(AVIOContext **s, URLContext *h)
 
 if (h->prot->url_read_seek)
 (*s)->seekable |= AVIO_SEEKABLE_TIME;
+
+if (h->prot->url_sync)
+(*s)->sync = io_sync;
 }
 (*s)->short_seek_get = io_short_seek;
 (*s)->av_class = _avio_class;
diff --git a/libavformat/url.h b/libavformat/url.h
index 4750bfff82..d032b45b65 100644
--- a/libavformat/url.h
+++ b/libavformat/url.h
@@ -97,6 +97,7 @@ typedef struct URLProtocol {
 int (*url_delete)(URLContext *h);
 int (*url_move)(URLContext *h_src, URLContext *h_dst);
 const char *default_whitelist;
+int (*url_sync)(URLContext *h);
 } URLProtocol;
 
 /**
@@ -228,6 +229,18 @@ int64_t ffurl_seek(URLContext *h, int64_t pos, int whence);
 int ffurl_closep(URLContext **h);
 int ffurl_close(URLContext *h);
 
+/**
+ * 

Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Remove global_sidx from movenc params for live streaming.

[Andrey Semashev]

On 12/4/18 11:49 AM, Jeyapal, Karthick wrote:


On 11/29/18 8:28 PM, Andrey Semashev wrote:

On 11/28/18 7:25 PM, Jeyapal, Karthick wrote:


On 11/28/18 4:41 PM, Andrey Semashev wrote:

The global_sidx flag causes errors like the following in movenc when media
segment removal is enabled via windos_size or remove_at_exit:

Non-consecutive fragments, writing incorrect sidx
Unable to re-open  output file for the second pass (faststart)

Removing global_sidx flag adds sidx atom to each moof fragment adding 
significant bitrate overhead.
Instead I have submitted a patch to handle this case cleanly in movenc. 
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-November/236873.html
Please try the above patch and let me know if that will work for you.


Yes, that patch seems to fix the errors. Thanks.

Hi Andrey,

That patch was not pushed due to some objections. I have submitted a new patch 
set addressing it.
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-December/237121.html
Could you please try the above patchset and confirm if everything works as 
expected?


I will test the final version. Though, if it doesn't change the effect 
since the last version then it should work as well.

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

Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Remove global_sidx from movenc params for live streaming.

[Andrey Semashev]

On 12/4/18 2:47 PM, Andrey Semashev wrote:

On 12/4/18 11:49 AM, Jeyapal, Karthick wrote:


On 11/29/18 8:28 PM, Andrey Semashev wrote:

On 11/28/18 7:25 PM, Jeyapal, Karthick wrote:


On 11/28/18 4:41 PM, Andrey Semashev wrote:
The global_sidx flag causes errors like the following in movenc 
when media

segment removal is enabled via windos_size or remove_at_exit:

Non-consecutive fragments, writing incorrect sidx
Unable to re-open  output file for the second pass (faststart)
Removing global_sidx flag adds sidx atom to each moof fragment 
adding significant bitrate overhead.
Instead I have submitted a patch to handle this case cleanly in 
movenc. 
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-November/236873.html

Please try the above patch and let me know if that will work for you.


Yes, that patch seems to fix the errors. Thanks.

Hi Andrey,

That patch was not pushed due to some objections. I have submitted a 
new patch set addressing it.

http://ffmpeg.org/pipermail/ffmpeg-devel/2018-December/237121.html
Could you please try the above patchset and confirm if everything 
works as expected?


I will test the final version. Though, if it doesn't change the effect 
since the last version then it should work as well.


I have tested v2 with the fixed flags setup as noted by Tobias and don't 
see any errors.

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

Re: [FFmpeg-devel] [PATCH v2] lavf/dashenc: Write media trailers when DASH trailer is written.

[Andrey Semashev]

On 12/3/18 8:45 AM, Jeyapal, Karthick wrote:


On 11/30/18 11:42 AM, Jeyapal, Karthick wrote:


On 11/29/18 11:58 PM, Andrey Semashev wrote:

This commit ensures that all (potentially, long) filesystem activity is
performed when the user calls av_write_trailer on the DASH libavformat
context, not when freeing the context. Also, this defers media segment
deletion until after the media trailers are written.
---
  libavformat/dashenc.c | 82 ++-
  1 file changed, 58 insertions(+), 24 deletions(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 6ce70e0076..ecfd84a32c 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -424,8 +424,6 @@ static void dash_free(AVFormatContext *s)
  return;
  for (i = 0; i < s->nb_streams; i++) {
  OutputStream *os = >streams[i];
-if (os->ctx && os->ctx_inited)
-av_write_trailer(os->ctx);
  if (os->ctx && os->ctx->pb)
  ffio_free_dyn_buf(>ctx->pb);
  ff_format_io_close(s, >out);
@@ -1331,6 +1329,47 @@ static void dashenc_delete_file(AVFormatContext *s, char 
*filename) {
  }
  }
  
+static int dashenc_delete_segment_file(AVFormatContext *s, const char* file)

+{
+DASHContext *c = s->priv_data;
+size_t dirname_len, file_len;
+char filename[1024];
+
+dirname_len = strlen(c->dirname);
+if (dirname_len >= sizeof(filename)) {
+av_log(s, AV_LOG_WARNING, "Cannot delete segments as the directory path is too long: 
%"PRIu64" characters: %s\n",
+(uint64_t)dirname_len, c->dirname);
+return AVERROR(ENAMETOOLONG);
+}
+
+memcpy(filename, c->dirname, dirname_len);
+
+file_len = strlen(file);
+if ((dirname_len + file_len) >= sizeof(filename)) {
+av_log(s, AV_LOG_WARNING, "Cannot delete segments as the path is too long: 
%"PRIu64" characters: %s%s\n",
+(uint64_t)(dirname_len + file_len), c->dirname, file);
+return AVERROR(ENAMETOOLONG);
+}
+
+memcpy(filename + dirname_len, file, file_len + 1); // include the 
terminating zero
+dashenc_delete_file(s, filename);
+
+return 0;
+}
+
+static inline void dashenc_delete_media_segments(AVFormatContext *s, 
OutputStream *os, int remove_count)
+{
+for (int i = 0; i < remove_count; ++i) {
+dashenc_delete_segment_file(s, os->segments[i]->file);
+
+// Delete the segment regardless of whether the file was successfully 
deleted
+av_free(os->segments[i]);
+}
+
+os->nb_segments -= remove_count;
+memmove(os->segments, os->segments + remove_count, os->nb_segments * 
sizeof(*os->segments));
+}
+
  static int dash_flush(AVFormatContext *s, int final, int stream)
  {
  DASHContext *c = s->priv_data;
@@ -1420,23 +1459,12 @@ static int dash_flush(AVFormatContext *s, int final, 
int stream)
  os->pos += range_length;
  }
  
-if (c->window_size || (final && c->remove_at_exit)) {

+if (c->window_size) {
  for (i = 0; i < s->nb_streams; i++) {
  OutputStream *os = >streams[i];
-int j;
-int remove = os->nb_segments - c->window_size - 
c->extra_window_size;
-if (final && c->remove_at_exit)
-remove = os->nb_segments;
-if (remove > 0) {
-for (j = 0; j < remove; j++) {
-char filename[1024];
-snprintf(filename, sizeof(filename), "%s%s", c->dirname, 
os->segments[j]->file);
-dashenc_delete_file(s, filename);
-av_free(os->segments[j]);
-}
-os->nb_segments -= remove;
-memmove(os->segments, os->segments + remove, os->nb_segments * 
sizeof(*os->segments));
-}
+int remove_count = os->nb_segments - c->window_size - 
c->extra_window_size;
+if (remove_count > 0)
+dashenc_delete_media_segments(s, os, remove_count);
  }
  }
  
@@ -1584,6 +1612,7 @@ static int dash_write_packet(AVFormatContext *s, AVPacket *pkt)

  static int dash_write_trailer(AVFormatContext *s)
  {
  DASHContext *c = s->priv_data;
+int i;
  
  if (s->nb_streams > 0) {

  OutputStream *os = >streams[0];
@@ -1599,14 +1628,19 @@ static int dash_write_trailer(AVFormatContext *s)
  }
  dash_flush(s, 1, -1);
  
-if (c->remove_at_exit) {

-char filename[1024];
-int i;
-for (i = 0; i < s->nb_streams; i++) {
-OutputStream *os = >streams[i];
-snprintf(filename, sizeof(filename), "%s%s", c->dirname, 
os->initfile);
-dashenc_delete_file(s,

[FFmpeg-devel] [PATCH 2/2] lavf/file: Add support for file syncing.

[Andrey Semashev]

This commit adds support for IO synchronization API to the file backend.
---
 libavformat/file.c   | 10 ++
 libavformat/os_support.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/libavformat/file.c b/libavformat/file.c
index 1d321c4205..9765fd76c7 100644
--- a/libavformat/file.c
+++ b/libavformat/file.c
@@ -254,6 +254,15 @@ static int64_t file_seek(URLContext *h, int64_t pos, int 
whence)
 return ret < 0 ? AVERROR(errno) : ret;
 }
 
+static int file_sync(URLContext *h)
+{
+if (h->flags & AVIO_FLAG_WRITE) {
+FileContext *c = h->priv_data;
+return fsync(c->fd);
+}
+return 0;
+}
+
 static int file_close(URLContext *h)
 {
 FileContext *c = h->priv_data;
@@ -353,6 +362,7 @@ const URLProtocol ff_file_protocol = {
 .url_close   = file_close,
 .url_get_file_handle = file_get_handle,
 .url_check   = file_check,
+.url_sync= file_sync,
 .url_delete  = file_delete,
 .url_move= file_move,
 .priv_data_size  = sizeof(FileContext),
diff --git a/libavformat/os_support.h b/libavformat/os_support.h
index 7a56dc9a7c..fcbdc884ba 100644
--- a/libavformat/os_support.h
+++ b/libavformat/os_support.h
@@ -93,6 +93,8 @@ static inline int is_dos_path(const char *path)
 #ifndef S_IWUSR
 #define S_IWUSR S_IWRITE
 #endif
+
+#define fsync _commit
 #endif
 
 #if CONFIG_NETWORK
-- 
2.19.1

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

[FFmpeg-devel] [PATCH 1/2] lavf: Add general API for IO buffer synchtonization.

[Andrey Semashev]

This commit adds a new set of functions to avio and url subsystems, which
allow users to invoke IO buffer synchronization with the underlying media.
The most obvious target for this extension if the filesystem streams. Invoking
IO synchronization allows user applications to ensure that all written content
has reached the filesystem on the media and can be observed by other processes.

The public API for this is avio_sync() function, which can be called on
AVIOContext. The internal, lower layer API is ffurl_sync(), which works
directly on the underlying URLContext. URLContext backends can add support for
this new API by setting the sync handler to the new url_sync member of
URLProtocol. When no handler is set then the sync operation is a no-op.
---
 libavformat/avio.c|  7 +++
 libavformat/avio.h| 16 
 libavformat/aviobuf.c | 17 +
 libavformat/url.h | 12 
 4 files changed, 52 insertions(+)

diff --git a/libavformat/avio.c b/libavformat/avio.c
index 663789ec02..19413ac586 100644
--- a/libavformat/avio.c
+++ b/libavformat/avio.c
@@ -623,6 +623,13 @@ int64_t ffurl_size(URLContext *h)
 return size;
 }
 
+int ffurl_sync(URLContext *h)
+{
+if (h->prot->url_sync)
+return h->prot->url_sync(h);
+return 0;
+}
+
 int ffurl_get_file_handle(URLContext *h)
 {
 if (!h || !h->prot || !h->prot->url_get_file_handle)
diff --git a/libavformat/avio.h b/libavformat/avio.h
index 75912ce6be..784cd1b509 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -349,6 +349,11 @@ typedef struct AVIOContext {
  * Try to buffer at least this amount of data before flushing it
  */
 int min_packet_size;
+
+/**
+ * A callback for synchronizing buffers with the media state.
+ */
+int (*sync)(void *opaque);
 } AVIOContext;
 
 /**
@@ -586,6 +591,17 @@ int avio_printf(AVIOContext *s, const char *fmt, ...) 
av_printf_format(2, 3);
  */
 void avio_flush(AVIOContext *s);
 
+/**
+ * Flush internal buffers and ensure the synchronized state of the
+ * resource associated with the context s. This call may be expensive.
+ * Not all resources support syncing, this operation is a no-op
+ * if sync is not supported or needed.
+ * This function can only be used if s was opened by avio_open().
+ *
+ * @return 0 on success, an AVERROR < 0 on error.
+ */
+int avio_sync(AVIOContext *s);
+
 /**
  * Read size bytes from AVIOContext into buf.
  * @return number of bytes read or AVERROR
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 5a33f82950..10be372bce 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -243,6 +243,14 @@ void avio_flush(AVIOContext *s)
 avio_seek(s, seekback, SEEK_CUR);
 }
 
+int avio_sync(AVIOContext *s)
+{
+avio_flush(s);
+if (s->sync)
+return s->sync(s->opaque);
+return 0;
+}
+
 int64_t avio_seek(AVIOContext *s, int64_t offset, int whence)
 {
 int64_t offset1;
@@ -978,6 +986,12 @@ static int64_t io_read_seek(void *opaque, int 
stream_index, int64_t timestamp, i
 return internal->h->prot->url_read_seek(internal->h, stream_index, 
timestamp, flags);
 }
 
+static int io_sync(void *opaque)
+{
+AVIOInternal *internal = opaque;
+return ffurl_sync(internal->h);
+}
+
 int ffio_fdopen(AVIOContext **s, URLContext *h)
 {
 AVIOInternal *internal = NULL;
@@ -1026,6 +1040,9 @@ int ffio_fdopen(AVIOContext **s, URLContext *h)
 
 if (h->prot->url_read_seek)
 (*s)->seekable |= AVIO_SEEKABLE_TIME;
+
+if (h->prot->url_sync)
+(*s)->sync = io_sync;
 }
 (*s)->short_seek_get = io_short_seek;
 (*s)->av_class = _avio_class;
diff --git a/libavformat/url.h b/libavformat/url.h
index 4750bfff82..26658695b0 100644
--- a/libavformat/url.h
+++ b/libavformat/url.h
@@ -97,6 +97,7 @@ typedef struct URLProtocol {
 int (*url_delete)(URLContext *h);
 int (*url_move)(URLContext *h_src, URLContext *h_dst);
 const char *default_whitelist;
+int (*url_sync)(URLContext *h);
 } URLProtocol;
 
 /**
@@ -228,6 +229,17 @@ int64_t ffurl_seek(URLContext *h, int64_t pos, int whence);
 int ffurl_closep(URLContext **h);
 int ffurl_close(URLContext *h);
 
+/**
+ * Flush any buffered data and synchronize the resource accessed
+ * by the URLContext h. This call may be expensive.
+ * Not all types of resources support syncing, the call is a no-op
+ * if sync is not supported or needed.
+ *
+ * @return a negative value if an error condition occurred, 0
+ * otherwise
+ */
+int ffurl_sync(URLContext *h);
+
 /**
  * Return the filesize of the resource accessed by h, AVERROR(ENOSYS)
  * if the operation is not supported by h, or another negative value
-- 
2.19.1

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

[FFmpeg-devel] [PATCH v3] lavf/dashenc: Write media trailers when DASH trailer is written.

[Andrey Semashev]

This commit ensures that all (potentially, long) filesystem activity is
performed when the user calls av_write_trailer on the DASH libavformat
context, not when freeing the context. Also, this defers media segment
deletion until after the media trailers are written.
---
 libavformat/dashenc.c | 85 ++-
 1 file changed, 60 insertions(+), 25 deletions(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 279a9bec54..4d9b564a94 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -441,8 +441,6 @@ static void dash_free(AVFormatContext *s)
 return;
 for (i = 0; i < s->nb_streams; i++) {
 OutputStream *os = >streams[i];
-if (os->ctx && os->ctx_inited)
-av_write_trailer(os->ctx);
 if (os->ctx && os->ctx->pb)
 ffio_free_dyn_buf(>ctx->pb);
 ff_format_io_close(s, >out);
@@ -1359,6 +1357,47 @@ static void dashenc_delete_file(AVFormatContext *s, char 
*filename) {
 }
 }
 
+static int dashenc_delete_segment_file(AVFormatContext *s, const char* file)
+{
+DASHContext *c = s->priv_data;
+size_t dirname_len, file_len;
+char filename[1024];
+
+dirname_len = strlen(c->dirname);
+if (dirname_len >= sizeof(filename)) {
+av_log(s, AV_LOG_WARNING, "Cannot delete segments as the directory 
path is too long: %"PRIu64" characters: %s\n",
+(uint64_t)dirname_len, c->dirname);
+return AVERROR(ENAMETOOLONG);
+}
+
+memcpy(filename, c->dirname, dirname_len);
+
+file_len = strlen(file);
+if ((dirname_len + file_len) >= sizeof(filename)) {
+av_log(s, AV_LOG_WARNING, "Cannot delete segments as the path is too 
long: %"PRIu64" characters: %s%s\n",
+(uint64_t)(dirname_len + file_len), c->dirname, file);
+return AVERROR(ENAMETOOLONG);
+}
+
+memcpy(filename + dirname_len, file, file_len + 1); // include the 
terminating zero
+dashenc_delete_file(s, filename);
+
+return 0;
+}
+
+static inline void dashenc_delete_media_segments(AVFormatContext *s, 
OutputStream *os, int remove_count)
+{
+for (int i = 0; i < remove_count; ++i) {
+dashenc_delete_segment_file(s, os->segments[i]->file);
+
+// Delete the segment regardless of whether the file was successfully 
deleted
+av_free(os->segments[i]);
+}
+
+os->nb_segments -= remove_count;
+memmove(os->segments, os->segments + remove_count, os->nb_segments * 
sizeof(*os->segments));
+}
+
 static int dash_flush(AVFormatContext *s, int final, int stream)
 {
 DASHContext *c = s->priv_data;
@@ -1448,23 +1487,12 @@ static int dash_flush(AVFormatContext *s, int final, 
int stream)
 os->pos += range_length;
 }
 
-if (c->window_size || (final && c->remove_at_exit)) {
+if (c->window_size) {
 for (i = 0; i < s->nb_streams; i++) {
 OutputStream *os = >streams[i];
-int j;
-int remove = os->nb_segments - c->window_size - 
c->extra_window_size;
-if (final && c->remove_at_exit)
-remove = os->nb_segments;
-if (remove > 0) {
-for (j = 0; j < remove; j++) {
-char filename[1024];
-snprintf(filename, sizeof(filename), "%s%s", c->dirname, 
os->segments[j]->file);
-dashenc_delete_file(s, filename);
-av_free(os->segments[j]);
-}
-os->nb_segments -= remove;
-memmove(os->segments, os->segments + remove, os->nb_segments * 
sizeof(*os->segments));
-}
+int remove_count = os->nb_segments - c->window_size - 
c->extra_window_size;
+if (remove_count > 0)
+dashenc_delete_media_segments(s, os, remove_count);
 }
 }
 
@@ -1615,6 +1643,7 @@ static int dash_write_packet(AVFormatContext *s, AVPacket 
*pkt)
 static int dash_write_trailer(AVFormatContext *s)
 {
 DASHContext *c = s->priv_data;
+int i;
 
 if (s->nb_streams > 0) {
 OutputStream *os = >streams[0];
@@ -1630,18 +1659,24 @@ static int dash_write_trailer(AVFormatContext *s)
 }
 dash_flush(s, 1, -1);
 
-if (c->remove_at_exit) {
-char filename[1024];
-int i;
-for (i = 0; i < s->nb_streams; i++) {
-OutputStream *os = >streams[i];
-snprintf(filename, sizeof(filename), "%s%s", c->dirname, 
os->initfile);
-dashenc_delete_file(s, filename);
+for (i = 0; i < s->nb_streams; ++i) {
+OutputStream *os = >streams[i];
+if (os->ctx && os->ctx_inited) {
+av_write_trailer(os->ctx);
 }
+
+if (c->remove_at_exit) {
+dashenc_delete_media_segments(s, os, os->nb_segments);
+dashenc_delete_segment_file(s, os->initfile);
+}
+}
+
+if (c->remove_at_exit) {
 dashenc_delete_file(s, s->url);
 
 if (c->hls_playlist && 

Re: [FFmpeg-devel] [PATCH 1/2] lavf: Add general API for IO buffer synchtonization.

[Andrey Semashev]

On 12/3/18 3:43 PM, Nicolas George wrote:

Andrey Semashev (2018-12-03):

This commit adds a new set of functions to avio and url subsystems, which
allow users to invoke IO buffer synchronization with the underlying media.
The most obvious target for this extension if the filesystem streams. Invoking
IO synchronization allows user applications to ensure that all written content
has reached the filesystem on the media and can be observed by other processes.

The public API for this is avio_sync() function, which can be called on
AVIOContext. The internal, lower layer API is ffurl_sync(), which works
directly on the underlying URLContext. URLContext backends can add support for
this new API by setting the sync handler to the new url_sync member of
URLProtocol. When no handler is set then the sync operation is a no-op.
---
  libavformat/avio.c|  7 +++
  libavformat/avio.h| 16 
  libavformat/aviobuf.c | 17 +
  libavformat/url.h | 12 
  4 files changed, 52 insertions(+)

diff --git a/libavformat/avio.c b/libavformat/avio.c
index 663789ec02..19413ac586 100644
--- a/libavformat/avio.c
+++ b/libavformat/avio.c
@@ -623,6 +623,13 @@ int64_t ffurl_size(URLContext *h)
  return size;
  }
  
+int ffurl_sync(URLContext *h)

+{
+if (h->prot->url_sync)
+return h->prot->url_sync(h);



+return 0;


I think it should be an error, probably ENOSUP. Otherwise, the
application will assume it was done.


I don't think this would be useful to users. I mean, the application can 
already check whether the operations is supported or not by inspecting 
url_sync. When the application doesn't care (which, I think, will be the 
majority of cases), they will treat ENOTSUP the same way as a success, 
so it only adds more burden to test for this error code.


I could use some separate "success" code for the "not supported" result, 
if there is one. Is there?

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

Re: [FFmpeg-devel] [PATCH 2/2] lavf/file: Add support for file syncing.

[Andrey Semashev]

On 12/3/18 3:46 PM, Nicolas George wrote:

Andrey Semashev (2018-12-03):

This commit adds support for IO synchronization API to the file backend.
---
  libavformat/file.c   | 10 ++
  libavformat/os_support.h |  2 ++
  2 files changed, 12 insertions(+)

diff --git a/libavformat/file.c b/libavformat/file.c
index 1d321c4205..9765fd76c7 100644
--- a/libavformat/file.c
+++ b/libavformat/file.c
@@ -254,6 +254,15 @@ static int64_t file_seek(URLContext *h, int64_t pos, int 
whence)
  return ret < 0 ? AVERROR(errno) : ret;
  }
  
+static int file_sync(URLContext *h)

+{
+if (h->flags & AVIO_FLAG_WRITE) {
+FileContext *c = h->priv_data;



+return fsync(c->fd);


In case of error, it needs to convert errno to an AVERROR code.


+}
+return 0;
+}
+
  static int file_close(URLContext *h)
  {
  FileContext *c = h->priv_data;
@@ -353,6 +362,7 @@ const URLProtocol ff_file_protocol = {
  .url_close   = file_close,
  .url_get_file_handle = file_get_handle,
  .url_check   = file_check,
+.url_sync= file_sync,
  .url_delete  = file_delete,
  .url_move= file_move,
  .priv_data_size  = sizeof(FileContext),
diff --git a/libavformat/os_support.h b/libavformat/os_support.h
index 7a56dc9a7c..fcbdc884ba 100644
--- a/libavformat/os_support.h
+++ b/libavformat/os_support.h
@@ -93,6 +93,8 @@ static inline int is_dos_path(const char *path)
  #ifndef S_IWUSR
  #define S_IWUSR S_IWRITE
  #endif
+



+#define fsync _commit


Defining with the arguments would be more robust. A few occasions in the
same file do not do that, they should.


  #endif
  
  #if CONFIG_NETWORK


Agreed to both comments.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Andrey Semashev]

On 12/6/18 8:07 AM, Karthick J wrote:

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

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

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

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

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


s->flags here should be mov->flags as well.


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



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

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

[Andrey Semashev]

On 12/6/18 12:20 PM, Andrey Semashev wrote:

On 12/6/18 8:07 AM, Karthick J wrote:

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

diff --git a/doc/muxers.texi b/doc/muxers.texi
index f1cc6f5fee..ca10741900 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -1325,6 +1325,10 @@ more efficient), but with this option set, the 
muxer writes one moof/mdat

  pair for each track, making it easier to separate tracks.
  This option is implicitly set when writing ismv (Smooth Streaming) 
files.

+@item -movflags skip_sidx
+Skip writing of sidx atom. When bitrate overhead due to sidx atom is 
high,

+this option could be used for cases where sidx atom is not mandatory.
+When global_sidx flag is enabled, this option will be ignored.
  @item -movflags faststart
  Run a second pass moving the index (moov atom) to the beginning of 
the file.
  This operation can take a while, and will not work in various 
situations such

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 6dab5193b0..2f7755bf69 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -75,6 +75,7 @@ static const AVOption options[] = {
  { "frag_discont", "Signal that the next fragment is 
discontinuous from earlier ones", 0, AV_OPT_TYPE_CONST, {.i64 = 
FF_MOV_FLAG_FRAG_DISCONT}, INT_MIN, INT_MAX, 
AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
  { "delay_moov", "Delay writing the initial moov until the first 
fragment is cut, or until the first fragment flush", 0, 
AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_DELAY_MOOV}, INT_MIN, INT_MAX, 
AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
  { "global_sidx", "Write a global sidx index at the start of the 
file", 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_GLOBAL_SIDX}, 
INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
+    { "skip_sidx", "Skip writing of sidx atom", 0, AV_OPT_TYPE_CONST, 
{.i64 = FF_MOV_FLAG_SKIP_SIDX}, INT_MIN, INT_MAX, 
AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
  { "write_colr", "Write colr atom (Experimental, may be renamed 
or changed, do not use from scripts)", 0, AV_OPT_TYPE_CONST, {.i64 = 
FF_MOV_FLAG_WRITE_COLR}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, 
"movflags" },
  { "write_gama", "Write deprecated gama atom", 0, 
AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_WRITE_GAMA}, INT_MIN, INT_MAX, 
AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
  { "use_metadata_tags", "Use mdta atom for metadata.", 0, 
AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_USE_MDTA}, INT_MIN, INT_MAX, 
AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
@@ -4603,7 +4604,8 @@ static int mov_write_moof_tag(AVIOContext *pb, 
MOVMuxContext *mov, int tracks,

  mov_write_moof_tag_internal(avio_buf, mov, tracks, 0);
  moof_size = ffio_close_null_buf(avio_buf);
-    if (mov->flags & FF_MOV_FLAG_DASH && !(mov->flags & 
FF_MOV_FLAG_GLOBAL_SIDX))

+    if (mov->flags & FF_MOV_FLAG_DASH &&
+    !(mov->flags & (FF_MOV_FLAG_GLOBAL_SIDX | 
FF_MOV_FLAG_SKIP_SIDX)))
  mov_write_sidx_tags(pb, mov, tracks, moof_size + 8 + 
mdat_size);
  if (mov->write_prft > MOV_PRFT_NONE && mov->write_prft < 
MOV_PRFT_NB)
@@ -5422,7 +5424,8 @@ int ff_mov_write_packet(AVFormatContext *s, 
AVPacket *pkt)
   * the next fragment. This means the cts of the first 
sample must
   * be the same in all fragments, unless end_pts was 
updated by

   * the packet causing the fragment to be written. */
-    if ((mov->flags & FF_MOV_FLAG_DASH && !(mov->flags & 
FF_MOV_FLAG_GLOBAL_SIDX)) ||

+    if ((mov->flags & FF_MOV_FLAG_DASH &&
+    !(mov->flags & (FF_MOV_FLAG_GLOBAL_SIDX | 
FF_MOV_FLAG_SKIP_SIDX))) ||

  mov->mode == MODE_ISM)
  pkt->pts = pkt->dts + trk->end_pts - 
trk->cluster[trk->entry].dts;

  } else {
@@ -6067,6 +6070,11 @@ static int mov_init(AVFormatContext *s)
  s->flags &= ~AVFMT_FLAG_AUTO_BSF;
  }
+    if (mov->flags & FF_MOV_FLAG_GLOBAL_SIDX && s->flags & 
FF_MOV_FLAG_SKIP_SIDX) {


s->flags here should be mov->flags as well.


Sorry, didn't read the later messages.

+    av_log(s, AV_LOG_WARNING, "Global SIDX enabled; Ignoring 
skip_sidx option\n");

+    mov->flags &= ~FF_MOV_FLAG_SKIP_SIDX;
+    }
+
  if (mov->flags & FF_MOV_FLAG_FASTSTART) {
  mov->reserved_moov_size = -1;
  }
diff --git a/libavformat/movenc.h b/libavformat/movenc.h
index fe605d1ad2..68d6f23a5a 100644
--- a/libavformat/movenc.h
+++ b/libavformat/movenc.h
@@ -257,6 +257,7 @@ typedef stru

Re: [FFmpeg-devel] [PATCH v2 1/2] lavf: Add general API for IO buffer synchronization.

[Andrey Semashev]

On 12/6/18 10:34 PM, Nicolas George wrote:

Andrey Semashev (2018-12-06):

Could you provide an example where ENOTSUP (i.e. the error code) would make
more sense for a sync operation, as opposed to AVIO_SYNC_NOT_SUPPORTED (i.e.
the success code)?


It is not a matter making more sense, both are semantically equivalent.
It is a matter of better matching the rest of the API.


I have used this API internally in an application for a few years, and I
never wanted to distinguish the "not supported" case from "success", let
alone specifically handle it as an error.


If you do not care that the sync was not done, then you did not need to
sync in the first place. Syncing is about guaranteeing the user that the
data is safe; if you ignore the information that it was not done, then
you are not guaranteeing it.


I do need sync - when it is supported by the underlying resource (e.g. a 
file). In that case, I care about it and I check for errors.


I do not care for the sync result if the underlying resource does not 
support the concept of syncing. In that case, I want to see a success 
code and treat the operation as a no-op.


With ENOTSUP, every call to avio_sync & co. I have would have to 
explicitly check for ENOTSUP and ignore it. I just don't see any benefit 
of this.

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

Re: [FFmpeg-devel] [PATCH v2 1/2] lavf: Add general API for IO buffer synchronization.

[Andrey Semashev]

On 12/6/18 9:29 PM, Nicolas George wrote:

Andrey Semashev (2018-12-04):

This commit adds a new set of functions to avio and url subsystems, which
allow users to invoke IO buffer synchronization with the underlying media.
The most obvious target for this extension if the filesystem streams. Invoking
IO synchronization allows user applications to ensure that all written content
has reached the filesystem on the media and can be observed by other processes.

The public API for this is avio_sync() function, which can be called on
AVIOContext. The internal, lower layer API is ffurl_sync(), which works
directly on the underlying URLContext. URLContext backends can add support for
this new API by setting the sync handler to the new url_sync member of
URLProtocol. When no handler is set then the sync operation is a no-op.
Users can distinguish this case from the successful completion by the result
code AVIO_SYNC_NOT_SUPPORTED, which is also considered a successful result
(i.e. >0).
---
  libavformat/avio.c|  7 +++
  libavformat/avio.h| 33 +
  libavformat/aviobuf.c | 17 +
  libavformat/url.h | 13 +
  4 files changed, 70 insertions(+)

diff --git a/libavformat/avio.c b/libavformat/avio.c
index 663789ec02..662d4ed971 100644
--- a/libavformat/avio.c
+++ b/libavformat/avio.c
@@ -623,6 +623,13 @@ int64_t ffurl_size(URLContext *h)
  return size;
  }
  
+int ffurl_sync(URLContext *h)

+{
+if (h->prot->url_sync)
+return h->prot->url_sync(h);
+return AVIO_SYNC_NOT_SUPPORTED;
+}
+
  int ffurl_get_file_handle(URLContext *h)
  {
  if (!h || !h->prot || !h->prot->url_get_file_handle)
diff --git a/libavformat/avio.h b/libavformat/avio.h
index 75912ce6be..403ee0fc7b 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -349,6 +349,15 @@ typedef struct AVIOContext {
   * Try to buffer at least this amount of data before flushing it
   */
  int min_packet_size;
+
+/**
+ * A callback for synchronizing buffers with the media state.
+ *
+ * @return AVIO_SYNC_SUCCESS on success, AVIO_SYNC_NOT_SUPPORTED if
+ * the operation is not supported and ignored, an AVERROR < 0
+ * on error.
+ */
+int (*sync)(void *opaque);
  } AVIOContext;
  
  /**

@@ -586,6 +595,30 @@ int avio_printf(AVIOContext *s, const char *fmt, ...) 
av_printf_format(2, 3);
   */
  void avio_flush(AVIOContext *s);
  



+/**
+ * Indicates that the sync operation has been carried out successfully
+ */
+#define AVIO_SYNC_SUCCESS 0
+
+/**
+ * Indicates that the sync operation is not supported by the underlying
+ * resource and has been ignored
+ */
+#define AVIO_SYNC_NOT_SUPPORTED 1


I must say, I really do not like this version at all. ENOTSUP feels like
a much more consistent solution.


Could you provide an example where ENOTSUP (i.e. the error code) would 
make more sense for a sync operation, as opposed to 
AVIO_SYNC_NOT_SUPPORTED (i.e. the success code)?


I have used this API internally in an application for a few years, and I 
never wanted to distinguish the "not supported" case from "success", let 
alone specifically handle it as an error. I have further patches to 
extend this functionality in ffmpeg and the intention there is similar - 
in no case I want the "not supported" case to be an error.

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

[FFmpeg-devel] [PATCH v2] lavf/dashenc: Use avpriv_io_delete to delete files.

[Andrey Semashev]

This fixes incorrect handling of file pseudo-URIs (i.e. when the filename
starts with "file:").
---
 libavformat/dashenc.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 6ce70e0076..412d7c8a21 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -1326,8 +1326,13 @@ static void dashenc_delete_file(AVFormatContext *s, char 
*filename) {
 
 av_dict_free(_opts);
 ff_format_io_close(s, );
-} else if (unlink(filename) < 0) {
-av_log(s, AV_LOG_ERROR, "failed to delete %s: %s\n", filename, 
strerror(errno));
+} else {
+int res = avpriv_io_delete(filename);
+if (res < 0) {
+char errbuf[AV_ERROR_MAX_STRING_SIZE];
+av_strerror(res, errbuf, sizeof(errbuf));
+av_log(s, (res == AVERROR(ENOENT) ? AV_LOG_WARNING : 
AV_LOG_ERROR), "failed to delete %s: %s\n", filename, errbuf);
+}
 }
 }
 
-- 
2.19.1

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

Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Write media trailers when DASH trailer is written.

[Andrey Semashev]

On 11/29/18 9:27 AM, Jeyapal, Karthick wrote:


On 11/28/18 5:13 PM, Andrey Semashev wrote:

This commit ensures that all (potentially, long) filesystem activity is
performed when the user calls av_write_trailer on the DASH libavformat
context, not when freeing the context. Also, this defers media segment
deletion until after the media trailers are written.
---
  libavformat/dashenc.c | 19 ++-
  1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 6ce70e0076..e1c959dc89 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -424,8 +424,6 @@ static void dash_free(AVFormatContext *s)
  return;
  for (i = 0; i < s->nb_streams; i++) {
  OutputStream *os = >streams[i];
-if (os->ctx && os->ctx_inited)
-av_write_trailer(os->ctx);
  if (os->ctx && os->ctx->pb)
  ffio_free_dyn_buf(>ctx->pb);
  ff_format_io_close(s, >out);
@@ -1420,13 +1418,11 @@ static int dash_flush(AVFormatContext *s, int final, 
int stream)
  os->pos += range_length;
  }
  
-if (c->window_size || (final && c->remove_at_exit)) {

+if (c->window_size) {
  for (i = 0; i < s->nb_streams; i++) {
  OutputStream *os = >streams[i];
  int j;
  int remove = os->nb_segments - c->window_size - 
c->extra_window_size;
-if (final && c->remove_at_exit)
-remove = os->nb_segments;

Is there any reason for deferring the delete after write_trailer.
Because if the file is getting deleted immediately, why should we bother about 
write_trailer? Is it causing any issues?
I am asking this, because the segment deletion code is getting duplicated due 
to this change. I am trying to avoid code duplication as much as possible.


This was partly in attempt to resolve the movenc errors caused by 
global_sidx. It did not completely remove the errors, but it seemed to 
have reduced them.


But mostly it is a logic sanity change. I believe it is incorrect to try 
writing a trailer to a non-existant file, and the downstream writer 
would be right to complain. If there is no file then you shouldn't be 
writing the trailer, but, AFAIK, that is not correct by libavformat 
usage protocol (i.e. you must write a trailer if you have written the 
header). Thus this change.


If you're worried about code duplication, I could move the segment 
deletion loop to a separate function.

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

Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Fix file URI handling when deleting files.

[Andrey Semashev]

On 11/28/18 7:47 PM, Jeyapal, Karthick wrote:


On 11/28/18 4:46 PM, Andrey Semashev wrote:

The URI used to open the output streams may be an actual URI with "file" scheme,
according to https://tools.ietf.org/html/rfc8089. This commit makes file
deletion routine recognize file URIs and extract the actual filesystem path
from it.

There is already some code in ffmpeg to handle this. It is present in 
file_delete() function in file.c.
We will need to avoid code duplication for the same functionality. One option 
could be to call avpriv_io_delete() function instead of calling unlink, so that 
file_delete function gets called.
Calling avpriv_io_delete will also make the delete functionality easily 
extendable for other output protocols.


That would be fine with me, but I'm using Linux. Looking at file_delete 
(in libavformat/file.c), it looks like it will only work on POSIX 
systems but not on Windows, since it doesn't have unistd.h. Am I 
correct? And if so, is avpriv_io_delete still the preferred approach?

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

Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Write media trailers when DASH trailer is written.

[Andrey Semashev]

On 11/29/18 9:27 AM, Jeyapal, Karthick wrote:


On 11/28/18 5:13 PM, Andrey Semashev wrote:

This commit ensures that all (potentially, long) filesystem activity is
performed when the user calls av_write_trailer on the DASH libavformat
context, not when freeing the context. Also, this defers media segment
deletion until after the media trailers are written.
---
  libavformat/dashenc.c | 19 ++-
  1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 6ce70e0076..e1c959dc89 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -424,8 +424,6 @@ static void dash_free(AVFormatContext *s)
  return;
  for (i = 0; i < s->nb_streams; i++) {
  OutputStream *os = >streams[i];
-if (os->ctx && os->ctx_inited)
-av_write_trailer(os->ctx);
  if (os->ctx && os->ctx->pb)
  ffio_free_dyn_buf(>ctx->pb);
  ff_format_io_close(s, >out);
@@ -1420,13 +1418,11 @@ static int dash_flush(AVFormatContext *s, int final, 
int stream)
  os->pos += range_length;
  }
  
-if (c->window_size || (final && c->remove_at_exit)) {

+if (c->window_size) {
  for (i = 0; i < s->nb_streams; i++) {
  OutputStream *os = >streams[i];
  int j;
  int remove = os->nb_segments - c->window_size - 
c->extra_window_size;
-if (final && c->remove_at_exit)
-remove = os->nb_segments;

Is there any reason for deferring the delete after write_trailer.
Because if the file is getting deleted immediately, why should we bother about 
write_trailer? Is it causing any issues?
I am asking this, because the segment deletion code is getting duplicated due 
to this change. I am trying to avoid code duplication as much as possible.

  if (remove > 0) {
  for (j = 0; j < remove; j++) {
  char filename[1024];
@@ -1599,11 +1595,24 @@ static int dash_write_trailer(AVFormatContext *s)
  }
  dash_flush(s, 1, -1);
  
+for (int i = 0; i < s->nb_streams; ++i) {

Can we merge this loop with the below loop for deleting init segments. The "if" 
condition for remove_at_exit, could be moved inside the merged loop.


I usually don't like conditions on constants inside loops, but I can do 
that.



+OutputStream *os = >streams[i];
+if (os->ctx && os->ctx_inited) {
+av_write_trailer(os->ctx);
+}
+}
+
  if (c->remove_at_exit) {
  char filename[1024];
  int i;
  for (i = 0; i < s->nb_streams; i++) {
  OutputStream *os = >streams[i];
+for (int j = 0; j < os->nb_segments; ++j) {
+snprintf(filename, sizeof(filename), "%s%s", c->dirname, 
os->segments[j]->file);
+dashenc_delete_file(s, filename);
+av_free(os->segments[j]);
+}
+os->nb_segments = 0;
  snprintf(filename, sizeof(filename), "%s%s", c->dirname, 
os->initfile);
  dashenc_delete_file(s, filename);
  }




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

Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Fix file URI handling when deleting files.

[Andrey Semashev]

On 11/29/18 2:15 PM, Andrey Semashev wrote:

On 11/28/18 7:47 PM, Jeyapal, Karthick wrote:


On 11/28/18 4:46 PM, Andrey Semashev wrote:
The URI used to open the output streams may be an actual URI with 
"file" scheme,

according to https://tools.ietf.org/html/rfc8089. This commit makes file
deletion routine recognize file URIs and extract the actual 
filesystem path

from it.
There is already some code in ffmpeg to handle this. It is present in 
file_delete() function in file.c.
We will need to avoid code duplication for the same functionality. One 
option could be to call avpriv_io_delete() function instead of calling 
unlink, so that file_delete function gets called.
Calling avpriv_io_delete will also make the delete functionality 
easily extendable for other output protocols.


That would be fine with me, but I'm using Linux. Looking at file_delete 
(in libavformat/file.c), it looks like it will only work on POSIX 
systems but not on Windows, since it doesn't have unistd.h. Am I 
correct? And if so, is avpriv_io_delete still the preferred approach?


Also, that code doesn't seem to support the URI with an authority field 
and doesn't check the special "localhost" case.

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

Re: [FFmpeg-devel] [PATCH v2 2/2] avformat/dashenc: Added an option to ignore io errors

[Andrey Semashev]

On 11/28/18 8:06 PM, Karthick J wrote:

When dashenc has to run for long duration(say 24x7 live stream), one can enable 
this option to ignore the io failure of few segment's upload due to an 
intermittent network issues.
When the network connection recovers dashenc will continue with the upload of 
the current segments, leading to the recovery of the stream.
---
  doc/muxers.texi   |  3 +++
  libavformat/dashenc.c | 17 +++--
  2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index a02ac01b55..f1cc6f5fee 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -300,6 +300,9 @@ If this flag is set, the dash segment files will be in in 
ISOBMFF format.
  @item webm
  If this flag is set, the dash segment files will be in in WebM format.
  
+@item -ignore_io_errors @var{ignore_io_errors}

+Ignore IO errors during open and write. Useful for long-duration runs with 
network output.
+
  @end table
  
  @anchor{framecrc}

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 2f403257c0..04218af6a6 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -138,6 +138,7 @@ typedef struct DASHContext {
  int index_correction;
  char *format_options_str;
  SegmentType segment_type_option;  /* segment type as specified in options 
*/
+int ignore_io_errors;
  } DASHContext;
  
  static struct codec_string {

@@ -846,7 +847,7 @@ static int write_manifest(AVFormatContext *s, int final)
  av_dict_free();
  if (ret < 0) {
  av_log(s, AV_LOG_ERROR, "Unable to open %s for writing\n", 
temp_filename);
-return ret;
+return c->ignore_io_errors ? 0 : ret;
  }
  out = c->mpd_out;
  avio_printf(out, "\n");
@@ -937,7 +938,7 @@ static int write_manifest(AVFormatContext *s, int final)
  av_dict_free();
  if (ret < 0) {
  av_log(s, AV_LOG_ERROR, "Unable to open %s for writing\n", 
temp_filename);
-return ret;
+return c->ignore_io_errors ? 0 : ret;
  }
  
  ff_hls_write_playlist_version(c->m3u8_out, 7);

@@ -1565,8 +1566,9 @@ static int dash_write_packet(AVFormatContext *s, AVPacket 
*pkt)
  set_http_options(, c);
  ret = dashenc_io_open(s, >out, os->temp_path, );
  av_dict_free();
-if (ret < 0)
-return ret;
+if (ret < 0) {


Please, add error logging as well:

char errbuf[AV_ERROR_MAX_STRING_SIZE];
av_strerror(ret, errbuf, sizeof(errbuf));
av_log(s, (c->ignore_io_errors ? AV_LOG_WARNING : 
AV_LOG_ERROR), "Unable to open %s for writing: %s\n", os->temp_path, 
errbuf);



+return c->ignore_io_errors ? 0 : ret;
+}
  }
  
  //write out the data immediately in streaming mode

@@ -1577,9 +1579,11 @@ static int dash_write_packet(AVFormatContext *s, 
AVPacket *pkt)
  write_styp(os->ctx->pb);
  avio_flush(os->ctx->pb);
  len = avio_get_dyn_buf (os->ctx->pb, );
-avio_write(os->out, buf + os->written_len, len - os->written_len);
+if (os->out) {
+avio_write(os->out, buf + os->written_len, len - os->written_len);
+avio_flush(os->out);
+}
  os->written_len = len;
-avio_flush(os->out);
  }
  
  return ret;

@@ -1670,6 +1674,7 @@ static const AVOption options[] = {
  { "auto", "select segment file format based on codec", 0, AV_OPT_TYPE_CONST, {.i64 
= SEGMENT_TYPE_AUTO }, 0, UINT_MAX,   E, "segment_type"},
  { "mp4", "make segment file in ISOBMFF format", 0, AV_OPT_TYPE_CONST, {.i64 = 
SEGMENT_TYPE_MP4 }, 0, UINT_MAX,   E, "segment_type"},
  { "webm", "make segment file in WebM format", 0, AV_OPT_TYPE_CONST, {.i64 = 
SEGMENT_TYPE_WEBM }, 0, UINT_MAX,   E, "segment_type"},
+{ "ignore_io_errors", "Ignore IO errors during open and write. Useful for 
long-duration runs with network output", OFFSET(ignore_io_errors), AV_OPT_TYPE_BOOL, { .i64 = 
0 }, 0, 1, E },
  { NULL },
  };
  



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

Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Remove global_sidx from movenc params for live streaming.

[Andrey Semashev]

On 11/28/18 7:25 PM, Jeyapal, Karthick wrote:


On 11/28/18 4:41 PM, Andrey Semashev wrote:

The global_sidx flag causes errors like the following in movenc when media
segment removal is enabled via windos_size or remove_at_exit:

Non-consecutive fragments, writing incorrect sidx
Unable to re-open  output file for the second pass (faststart)

Removing global_sidx flag adds sidx atom to each moof fragment adding 
significant bitrate overhead.
Instead I have submitted a patch to handle this case cleanly in movenc. 
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-November/236873.html
Please try the above patch and let me know if that will work for you.


Yes, that patch seems to fix the errors. Thanks.

On a slightly unrelated note, during testing, I've also seen different 
errors like these:


[AVFormatContext] Application provided duration: -1 / timestamp: 243456 
is out of range for mov/mp4 format

[AVFormatContext] pts has no value

These happen when media segments are rotated and file deletion is 
enabled. They don't seem to happen on every file rotation, though, and I 
can't find what could be causing them. These errors are present 
regardless of the global_sidx flag or your movenc patch. Do you have an 
idea what could be causing them?

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

Re: [FFmpeg-devel] [PATCH] lavf/dashenc: Fix file URI handling when deleting files.

[Andrey Semashev]

On 11/29/18 2:17 PM, Andrey Semashev wrote:

On 11/29/18 2:15 PM, Andrey Semashev wrote:

On 11/28/18 7:47 PM, Jeyapal, Karthick wrote:


On 11/28/18 4:46 PM, Andrey Semashev wrote:
The URI used to open the output streams may be an actual URI with 
"file" scheme,
according to https://tools.ietf.org/html/rfc8089. This commit makes 
file
deletion routine recognize file URIs and extract the actual 
filesystem path

from it.
There is already some code in ffmpeg to handle this. It is present in 
file_delete() function in file.c.
We will need to avoid code duplication for the same functionality. 
One option could be to call avpriv_io_delete() function instead of 
calling unlink, so that file_delete function gets called.
Calling avpriv_io_delete will also make the delete functionality 
easily extendable for other output protocols.


That would be fine with me, but I'm using Linux. Looking at 
file_delete (in libavformat/file.c), it looks like it will only work 
on POSIX systems but not on Windows, since it doesn't have unistd.h. 
Am I correct? And if so, is avpriv_io_delete still the preferred 
approach?


Also, that code doesn't seem to support the URI with an authority field 
and doesn't check the special "localhost" case.


I've sent a new set of patches that updates both file.c and dashenc.c.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 2/2] lavf/dashenc: Use avpriv_io_delete to delete files.

[Andrey Semashev]

This fixes incorrect handling of file URIs (i.e. when the filename starts
with "file:", possibly followed by URI authority).
---
 libavformat/dashenc.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 6ce70e0076..426dc91bcc 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -25,6 +25,7 @@
 #include 
 #endif
 
+#include "libavutil/error.h"
 #include "libavutil/avassert.h"
 #include "libavutil/avutil.h"
 #include "libavutil/avstring.h"
@@ -1326,8 +1327,13 @@ static void dashenc_delete_file(AVFormatContext *s, char 
*filename) {
 
 av_dict_free(_opts);
 ff_format_io_close(s, );
-} else if (unlink(filename) < 0) {
-av_log(s, AV_LOG_ERROR, "failed to delete %s: %s\n", filename, 
strerror(errno));
+} else {
+int res = avpriv_io_delete(filename);
+if (res < 0) {
+char errbuf[AV_ERROR_MAX_STRING_SIZE];
+av_strerror(res, errbuf, sizeof(errbuf));
+av_log(s, (res == AVERROR(ENOENT) ? AV_LOG_WARNING : 
AV_LOG_ERROR), "failed to delete %s: %s\n", filename, errbuf);
+}
 }
 }
 
-- 
2.19.1

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

[FFmpeg-devel] [PATCH 1/2] lavf/file: Add proper support for authority in file URIs.

[Andrey Semashev]

Previously, URIs with authority field were incorrectly interpreted as if
the authority was part of the path. This commit adds more complete file URI
parsing according to https://tools.ietf.org/html/rfc8089. In particular, the
file backend now recognizes URIs with authority field and ensures that it is
either empty or contains the special value "localhost". The file backend will
return EINVAL if the user attempts to use it with a URI referring to
a remote host.

Also, enable file_delete() on Windows as it provides equivalents of unlink()
and rmdir(). The compatibility glue is already provided by os_support.h.
---
 libavformat/file.c | 55 --
 1 file changed, 48 insertions(+), 7 deletions(-)

diff --git a/libavformat/file.c b/libavformat/file.c
index 1d321c4205..040197d50d 100644
--- a/libavformat/file.c
+++ b/libavformat/file.c
@@ -19,6 +19,8 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include "libavutil/log.h"
+#include "libavutil/error.h"
 #include "libavutil/avstring.h"
 #include "libavutil/internal.h"
 #include "libavutil/opt.h"
@@ -104,6 +106,31 @@ static const AVClass pipe_class = {
 .version= LIBAVUTIL_VERSION_INT,
 };
 
+static int file_get_path(const char* filename, const char** ppath)
+{
+const char* path = filename;
+// Check if the filename is a file URI. 
https://tools.ietf.org/html/rfc8089#section-2
+if (av_strncasecmp(path, "file:", sizeof("file:") - 1) == 0) {
+path += sizeof("file:") - 1;
+if (path[0] == '/' && path[1] == '/') {
+// The URI may have an authority part. Check that the authority 
does not contain
+// a remote host name. We cannot access filesystem on a different 
host.
+path += 2;
+if (path[0] != '/') {
+if (strncmp(path, "localhost/", sizeof("localhost/") - 1) == 
0) {
+path += sizeof("localhost") - 1;
+} else {
+av_log(NULL, AV_LOG_ERROR, "File URIs referencing a remote 
host are not supported: %s\n", filename);
+return AVERROR(EINVAL);
+}
+}
+}
+}
+
+*ppath = path;
+return 0;
+}
+
 static int file_read(URLContext *h, unsigned char *buf, int size)
 {
 FileContext *c = h->priv_data;
@@ -136,7 +163,9 @@ static int file_check(URLContext *h, int mask)
 {
 int ret = 0;
 const char *filename = h->filename;
-av_strstart(filename, "file:", );
+ret = file_get_path(filename, );
+if (ret < 0)
+return ret;
 
 {
 #if HAVE_ACCESS && defined(R_OK)
@@ -167,10 +196,12 @@ static int file_check(URLContext *h, int mask)
 
 static int file_delete(URLContext *h)
 {
-#if HAVE_UNISTD_H
+#if HAVE_UNISTD_H || defined(_WIN32)
 int ret;
 const char *filename = h->filename;
-av_strstart(filename, "file:", );
+ret = file_get_path(filename, );
+if (ret < 0)
+return ret;
 
 ret = rmdir(filename);
 if (ret < 0 && errno == ENOTDIR)
@@ -188,8 +219,12 @@ static int file_move(URLContext *h_src, URLContext *h_dst)
 {
 const char *filename_src = h_src->filename;
 const char *filename_dst = h_dst->filename;
-av_strstart(filename_src, "file:", _src);
-av_strstart(filename_dst, "file:", _dst);
+int ret = file_get_path(filename_src, _src);
+if (ret < 0)
+return ret;
+ret = file_get_path(filename_dst, _dst);
+if (ret < 0)
+return ret;
 
 if (rename(filename_src, filename_dst) < 0)
 return AVERROR(errno);
@@ -206,7 +241,9 @@ static int file_open(URLContext *h, const char *filename, 
int flags)
 int fd;
 struct stat st;
 
-av_strstart(filename, "file:", );
+int ret = file_get_path(filename, );
+if (ret < 0)
+return ret;
 
 if (flags & AVIO_FLAG_WRITE && flags & AVIO_FLAG_READ) {
 access = O_CREAT | O_RDWR;
@@ -264,8 +301,12 @@ static int file_open_dir(URLContext *h)
 {
 #if HAVE_LSTAT
 FileContext *c = h->priv_data;
+const char* dirname = h->filename;
+int ret = file_get_path(dirname, );
+if (ret < 0)
+return ret;
 
-c->dir = opendir(h->filename);
+c->dir = opendir(dirname);
 if (!c->dir)
 return AVERROR(errno);
 
-- 
2.19.1

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

Re: [FFmpeg-devel] [PATCH 4/4] lavf/dashenc: Fix AVDictionary leaks in case of various init errors.

[Andrey Semashev]

On Wed, Nov 21, 2018 at 3:45 PM Jeyapal, Karthick  wrote:
> On 11/20/18 6:01 PM, Andrey Semashev wrote:
> > On 11/18/18 1:55 PM, Jeyapal, Karthick wrote:
> > > Thanks for sending these excellent patches. The entire patchset looks 
> > > good to me.
> > > Also, many thanks for your patience and taking the earlier review 
> > > comments in the right spirit.
> >
> > Ping for merging it then?
> Thanks for the ping. Pushed all the dashenc patches.
> Left out the dashdec patch, as its maintainer has not commented yet. Maybe we 
> can wait for few more days before pushing the dashdec patch.

Looks like noone will comment on the dashdec patch. Could you merge it, please?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 4/4] lavf/dashenc: Fix AVDictionary leaks in case of various init errors.

[Andrey Semashev]

On 11/18/18 1:55 PM, Jeyapal, Karthick wrote:

Thanks for sending these excellent patches. The entire patchset looks good to 
me.
Also, many thanks for your patience and taking the earlier review comments in 
the right spirit.


Ping for merging it then?


On 11/17/18 11:10 PM, Andrey Semashev wrote:

---
  libavformat/dashenc.c | 13 +++--
  1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index f552503564..2c872f93a1 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -833,12 +833,12 @@ static int write_manifest(AVFormatContext *s, int final)
  snprintf(temp_filename, sizeof(temp_filename), use_rename ? "%s.tmp" : "%s", 
s->url);
  set_http_options(, c);
  ret = dashenc_io_open(s, >mpd_out, temp_filename, );
+av_dict_free();
  if (ret < 0) {
  av_log(s, AV_LOG_ERROR, "Unable to open %s for writing\n", 
temp_filename);
  return ret;
  }
  out = c->mpd_out;
-av_dict_free();
  avio_printf(out, "\n");
  avio_printf(out, "http://www.w3.org/2001/XMLSchema-instance\"\n;
  "\txmlns=\"urn:mpeg:dash:schema:mpd:2011\"\n"
@@ -924,11 +924,11 @@ static int write_manifest(AVFormatContext *s, int final)
  
  set_http_options(, c);

  ret = dashenc_io_open(s, >m3u8_out, temp_filename, );
+av_dict_free();
  if (ret < 0) {
  av_log(s, AV_LOG_ERROR, "Unable to open %s for writing\n", 
temp_filename);
  return ret;
  }
-av_dict_free();
  
  ff_hls_write_playlist_version(c->m3u8_out, 7);
  
@@ -1122,9 +1122,9 @@ static int dash_init(AVFormatContext *s)

  snprintf(filename, sizeof(filename), "%s%s", c->dirname, 
os->initfile);
  set_http_options(, c);
  ret = s->io_open(s, >out, filename, AVIO_FLAG_WRITE, );
+av_dict_free();
  if (ret < 0)
  return ret;
-av_dict_free();
  os->init_start_pos = 0;
  
  if (c->format_options_str) {

@@ -1145,11 +1145,12 @@ static int dash_init(AVFormatContext *s)
  av_dict_set_int(, "dash_track_number", i + 1, 0);
  av_dict_set_int(, "live", 1, 0);
  }
-if ((ret = avformat_init_output(ctx, )) < 0)
+ret = avformat_init_output(ctx, );
+av_dict_free();
+if (ret < 0)
  return ret;
  os->ctx_inited = 1;
  avio_flush(ctx->pb);
-av_dict_free();
  
  av_log(s, AV_LOG_VERBOSE, "Representation %d init segment will be written to: %s\n", i, filename);
  
@@ -1553,9 +1554,9 @@ static int dash_write_packet(AVFormatContext *s, AVPacket *pkt)

   use_rename ? "%s.tmp" : "%s", os->full_path);
  set_http_options(, c);
  ret = dashenc_io_open(s, >out, os->temp_path, );
+av_dict_free();
  if (ret < 0)
  return ret;
-av_dict_free();
  }
  
  //write out the data immediately in streaming mode




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

[FFmpeg-devel] [PATCH] lavf/dashenc: Fix segment duration overflow on fine time bases.

[Andrey Semashev]

When stream time bases are very fine grained (e.g. nanoseconds), 32-bit
segment duration may overflow for even for rather small segment duration
(about 4 seconds long). Therefore we use 64-bit values for segment duration.
---
 libavformat/dashenc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index d151921175..8d0bc4baa2 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -59,7 +59,7 @@ typedef struct Segment {
 int64_t start_pos;
 int range_length, index_length;
 int64_t time;
-int duration;
+int64_t duration;
 int n;
 } Segment;
 
@@ -428,7 +428,7 @@ static void output_segment_list(OutputStream *os, 
AVIOContext *out, AVFormatCont
 cur_time = seg->time;
 avio_printf(out, "t=\"%"PRId64"\" ", seg->time);
 }
-avio_printf(out, "d=\"%d\" ", seg->duration);
+avio_printf(out, "d=\"%"PRId64"\" ", seg->duration);
 while (i + repeat + 1 < os->nb_segments &&
os->segments[i + repeat + 1]->duration == seg->duration 
&&
os->segments[i + repeat + 1]->time == os->segments[i + 
repeat]->time + os->segments[i + repeat]->duration)
@@ -1149,7 +1149,7 @@ static int dash_write_header(AVFormatContext *s)
 }
 
 static int add_segment(OutputStream *os, const char *file,
-   int64_t time, int duration,
+   int64_t time, int64_t duration,
int64_t start_pos, int64_t range_length,
int64_t index_length, int next_exp_index)
 {
-- 
2.19.1

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

[FFmpeg-devel] [PATCH 1/4] lavf/dashenc: Add DASH segment type auto and make it the default

[Andrey Semashev]

This commit restores the ability to create DASH streams with codecs
that require different containers that was lost after commit
2efdbf7367989cf9d296c25fa3d2aff8d6e25fdd. It adds a new "auto" value for
the dash_segment_type option and makes it the default. When in this mode,
the segment format will be chosen based on the codec used in the stream:
webm for Vorbis, Opus, VP8 or VP9, mp4 otherwise.
---
 doc/muxers.texi   |  7 +++--
 libavformat/dashenc.c | 72 ---
 2 files changed, 59 insertions(+), 20 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 62f4091e31..2fed5cf3d4 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -289,10 +289,13 @@ Set container format (mp4/webm) options using a @code{:} 
separated list of
 key=value parameters. Values containing @code{:} special characters must be
 escaped.
 
-@item dash_segment_type @var{dash_segment_type}
+@item -dash_segment_type @var{dash_segment_type}
 Possible values:
+@item auto
+If this flag is set, the dash segment files format will be selected based on 
the stream codec. This is the default mode.
+
 @item mp4
-If this flag is set, the dash segment files will be in in ISOBMFF format. This 
is the default format.
+If this flag is set, the dash segment files will be in in ISOBMFF format.
 
 @item webm
 If this flag is set, the dash segment files will be in in WebM format.
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index d151921175..0af7b85c5f 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -49,7 +49,8 @@
 #include "dash.h"
 
 typedef enum {
-SEGMENT_TYPE_MP4 = 0,
+SEGMENT_TYPE_AUTO = 0,
+SEGMENT_TYPE_MP4,
 SEGMENT_TYPE_WEBM,
 SEGMENT_TYPE_NB
 } SegmentType;
@@ -84,6 +85,8 @@ typedef struct OutputStream {
 int64_t first_pts, start_pts, max_pts;
 int64_t last_dts, last_pts;
 int bit_rate;
+SegmentType segment_type;  /* segment type selected for this particular 
stream */
+const char *format_name;
 
 char codec_str[100];
 int written_len;
@@ -131,8 +134,7 @@ typedef struct DASHContext {
 int64_t timeout;
 int index_correction;
 char *format_options_str;
-SegmentType segment_type;
-const char *format_name;
+SegmentType segment_type_option;  /* segment type as specified in options 
*/
 } DASHContext;
 
 static struct codec_string {
@@ -151,6 +153,7 @@ static struct format_string {
 SegmentType segment_type;
 const char *str;
 } formats[] = {
+{ SEGMENT_TYPE_AUTO, "auto" },
 { SEGMENT_TYPE_MP4, "mp4" },
 { SEGMENT_TYPE_WEBM, "webm" },
 { 0, NULL }
@@ -197,6 +200,38 @@ static const char *get_format_str(SegmentType 
segment_type) {
 return NULL;
 }
 
+static inline SegmentType select_segment_type(SegmentType segment_type, 
AVCodecID codec_id)
+{
+if (segment_type == SEGMENT_TYPE_AUTO) {
+if (codec_id == AV_CODEC_ID_OPUS || codec_id == AV_CODEC_ID_VORBIS ||
+codec_id == AV_CODEC_ID_VP8 || codec_id == AV_CODEC_ID_VP9) {
+segment_type = SEGMENT_TYPE_WEBM;
+} else {
+segment_type = SEGMENT_TYPE_MP4;
+}
+}
+
+return segment_type;
+}
+
+static int init_segment_types(AVFormatContext *s)
+{
+DASHContext *c = s->priv_data;
+for (int i = 0; i < s->nb_streams; ++i) {
+OutputStream *os = >streams[i];
+SegmentType segment_type = select_segment_type(
+c->segment_type_option, s->streams[i]->codecpar->codec_id);
+os->segment_type = segment_type;
+os->format_name = get_format_str(segment_type);
+if (!os->format_name) {
+av_log(s, AV_LOG_ERROR, "Could not select DASH segment type for 
stream %d\n", i);
+return AVERROR_MUXER_NOT_FOUND;
+}
+}
+
+return 0;
+}
+
 static int check_file_extension(const char *filename, const char *extension) {
 char *dot;
 if (!filename || !extension)
@@ -622,13 +657,13 @@ static int write_adaptation_set(AVFormatContext *s, 
AVIOContext *out, int as_ind
 if (as->media_type == AVMEDIA_TYPE_VIDEO) {
 AVStream *st = s->streams[i];
 avio_printf(out, "\t\t\tformat_name, os->codec_str, bandwidth_str, 
s->streams[i]->codecpar->width, s->streams[i]->codecpar->height);
+i, os->format_name, os->codec_str, bandwidth_str, 
s->streams[i]->codecpar->width, s->streams[i]->codecpar->height);
 if (st->avg_frame_rate.num)
 avio_printf(out, " frameRate=\"%d/%d\"", 
st->avg_frame_rate.num, st->avg_frame_rate.den);
 avio_printf(out, ">\n");
 } else {
 avio_printf(out, "\t\t\t\n",
-i, c->format_name, os->codec_str, bandwidth_str, 
s->streams[i]->codecpar->sample_rate);
+i, os->format_name, os->codec_str, bandwidth_str, 
s->streams[i]->codecpar->sample_rate);
 avio_printf(out, "\t\t\t\t\n",
 s->streams[i]->codecpar->channels);
 }
@@ 

[FFmpeg-devel] [PATCH 3/4] lavf/dashdec: Add webm to the list of allowed extensions.

[Andrey Semashev]

This is in coherence with dashenc, which can now generate segments with
webm file name extension by default. Dashdec should be able to handle
such streams by default as well.
---
 libavformat/dashdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 497e7e469c..cf603bf075 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -2242,7 +2242,7 @@ static int dash_probe(AVProbeData *p)
 static const AVOption dash_options[] = {
 {"allowed_extensions", "List of file extensions that dash is allowed to 
access",
 OFFSET(allowed_extensions), AV_OPT_TYPE_STRING,
-{.str = "aac,m4a,m4s,m4v,mov,mp4"},
+{.str = "aac,m4a,m4s,m4v,mov,mp4,webm"},
 INT_MIN, INT_MAX, FLAGS},
 {NULL}
 };
-- 
2.19.1

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

[FFmpeg-devel] [PATCH 2/4] lavf/dashenc: Add support for format-specific file extensions.

[Andrey Semashev]

The file name template options now support a new "$ext$" placeholder,
which is replaced with a filename extension specific for the selected
file format. This is useful for the new "auto" format mode, when
different streams may use different file formats, and it is not
possible to specify the correct file name extension exactly.

Resolves warnings in the log about webm segments not having webm extensions.
---
 doc/muxers.texi   |  6 +++---
 libavformat/dashenc.c | 48 +++
 2 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 2fed5cf3d4..a02ac01b55 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -245,11 +245,11 @@ Enable (1) or disable (0) use of SegmentTimeline in 
SegmentTemplate.
 @item -single_file @var{single_file}
 Enable (1) or disable (0) storing all segments in one file, accessed using 
byte ranges.
 @item -single_file_name @var{file_name}
-DASH-templated name to be used for baseURL. Implies @var{single_file} set to 
"1".
+DASH-templated name to be used for baseURL. Implies @var{single_file} set to 
"1". In the template, "$ext$" is replaced with the file name extension specific 
for the segment format.
 @item -init_seg_name @var{init_name}
-DASH-templated name to used for the initialization segment. Default is 
"init-stream$RepresentationID$.m4s"
+DASH-templated name to used for the initialization segment. Default is 
"init-stream$RepresentationID$.$ext$". "$ext$" is replaced with the file name 
extension specific for the segment format.
 @item -media_seg_name @var{segment_name}
-DASH-templated name to used for the media segments. Default is 
"chunk-stream$RepresentationID$-$Number%05d$.m4s"
+DASH-templated name to used for the media segments. Default is 
"chunk-stream$RepresentationID$-$Number%05d$.$ext$". "$ext$" is replaced with 
the file name extension specific for the segment format.
 @item -utc_timing_url @var{utc_url}
 URL of the page that will return the UTC timestamp in ISO format. Example: 
"https://time.akamai.com/?iso;
 @item method @var{method}
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 0af7b85c5f..f552503564 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -87,6 +87,9 @@ typedef struct OutputStream {
 int bit_rate;
 SegmentType segment_type;  /* segment type selected for this particular 
stream */
 const char *format_name;
+const char *single_file_name;  /* file names selected for this particular 
stream */
+const char *init_seg_name;
+const char *media_seg_name;
 
 char codec_str[100];
 int written_len;
@@ -119,7 +122,7 @@ typedef struct DASHContext {
 int64_t total_duration;
 char availability_start_time[100];
 char dirname[1024];
-const char *single_file_name;
+const char *single_file_name;  /* file names as specified in options */
 const char *init_seg_name;
 const char *media_seg_name;
 const char *utc_timing_url;
@@ -200,7 +203,7 @@ static const char *get_format_str(SegmentType segment_type) 
{
 return NULL;
 }
 
-static inline SegmentType select_segment_type(SegmentType segment_type, 
AVCodecID codec_id)
+static inline SegmentType select_segment_type(SegmentType segment_type, enum 
AVCodecID codec_id)
 {
 if (segment_type == SEGMENT_TYPE_AUTO) {
 if (codec_id == AV_CODEC_ID_OPUS || codec_id == AV_CODEC_ID_VORBIS ||
@@ -425,6 +428,9 @@ static void dash_free(AVFormatContext *s)
 for (j = 0; j < os->nb_segments; j++)
 av_free(os->segments[j]);
 av_free(os->segments);
+av_freep(>single_file_name);
+av_freep(>init_seg_name);
+av_freep(>media_seg_name);
 }
 av_freep(>streams);
 
@@ -451,7 +457,7 @@ static void output_segment_list(OutputStream *os, 
AVIOContext *out, AVFormatCont
 avio_printf(out, "availabilityTimeOffset=\"%.3f\" ",
 os->availability_time_offset);
 }
-avio_printf(out, "initialization=\"%s\" media=\"%s\" 
startNumber=\"%d\">\n", c->init_seg_name, c->media_seg_name, c->use_timeline ? 
start_number : 1);
+avio_printf(out, "initialization=\"%s\" media=\"%s\" 
startNumber=\"%d\">\n", os->init_seg_name, os->media_seg_name, c->use_timeline 
? start_number : 1);
 if (c->use_timeline) {
 int64_t cur_time = 0;
 avio_printf(out, "\t\t\t\t\t\n");
@@ -1056,10 +1062,26 @@ static int dash_init(AVFormatContext *s)
 if (!ctx)
 return AVERROR(ENOMEM);
 
+if (c->init_seg_name) {
+os->init_seg_name = av_strireplace(c->init_seg_name, "$ext$", 
os->format_name);
+if (!os->init_seg_name)
+return AVERROR(ENOMEM);
+}
+if (c->media_seg_name) {
+os->media_seg_name = av_strireplace(c->media_seg_name, "$ext$", 
os->format_name);
+if (!os->media_seg_name)
+return AVERROR(ENOMEM);
+}
+if 

[FFmpeg-devel] [PATCH 4/4] lavf/dashenc: Fix AVDictionary leaks in case of various init errors.

[Andrey Semashev]

---
 libavformat/dashenc.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index f552503564..2c872f93a1 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -833,12 +833,12 @@ static int write_manifest(AVFormatContext *s, int final)
 snprintf(temp_filename, sizeof(temp_filename), use_rename ? "%s.tmp" : 
"%s", s->url);
 set_http_options(, c);
 ret = dashenc_io_open(s, >mpd_out, temp_filename, );
+av_dict_free();
 if (ret < 0) {
 av_log(s, AV_LOG_ERROR, "Unable to open %s for writing\n", 
temp_filename);
 return ret;
 }
 out = c->mpd_out;
-av_dict_free();
 avio_printf(out, "\n");
 avio_printf(out, "http://www.w3.org/2001/XMLSchema-instance\"\n;
 "\txmlns=\"urn:mpeg:dash:schema:mpd:2011\"\n"
@@ -924,11 +924,11 @@ static int write_manifest(AVFormatContext *s, int final)
 
 set_http_options(, c);
 ret = dashenc_io_open(s, >m3u8_out, temp_filename, );
+av_dict_free();
 if (ret < 0) {
 av_log(s, AV_LOG_ERROR, "Unable to open %s for writing\n", 
temp_filename);
 return ret;
 }
-av_dict_free();
 
 ff_hls_write_playlist_version(c->m3u8_out, 7);
 
@@ -1122,9 +1122,9 @@ static int dash_init(AVFormatContext *s)
 snprintf(filename, sizeof(filename), "%s%s", c->dirname, os->initfile);
 set_http_options(, c);
 ret = s->io_open(s, >out, filename, AVIO_FLAG_WRITE, );
+av_dict_free();
 if (ret < 0)
 return ret;
-av_dict_free();
 os->init_start_pos = 0;
 
 if (c->format_options_str) {
@@ -1145,11 +1145,12 @@ static int dash_init(AVFormatContext *s)
 av_dict_set_int(, "dash_track_number", i + 1, 0);
 av_dict_set_int(, "live", 1, 0);
 }
-if ((ret = avformat_init_output(ctx, )) < 0)
+ret = avformat_init_output(ctx, );
+av_dict_free();
+if (ret < 0)
 return ret;
 os->ctx_inited = 1;
 avio_flush(ctx->pb);
-av_dict_free();
 
 av_log(s, AV_LOG_VERBOSE, "Representation %d init segment will be 
written to: %s\n", i, filename);
 
@@ -1553,9 +1554,9 @@ static int dash_write_packet(AVFormatContext *s, AVPacket 
*pkt)
  use_rename ? "%s.tmp" : "%s", os->full_path);
 set_http_options(, c);
 ret = dashenc_io_open(s, >out, os->temp_path, );
+av_dict_free();
 if (ret < 0)
 return ret;
-av_dict_free();
 }
 
 //write out the data immediately in streaming mode
-- 
2.19.1

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

[FFmpeg-devel] [PATCH v3 1/2] lavf: Add general API for IO buffer synchronization.

[Andrey Semashev]

This commit adds a new set of functions to avio and url subsystems, which
allow users to invoke IO buffer synchronization with the underlying media.
The most obvious target for this extension if the filesystem streams. Invoking
IO synchronization allows user applications to ensure that all written content
has reached the filesystem on the media and can be observed by other processes.

The public API for this is avio_sync() function, which can be called on
AVIOContext. The internal, lower layer API is ffurl_sync(), which works
directly on the underlying URLContext. URLContext backends can add support for
this new API by setting the sync handler to the new url_sync member of
URLProtocol. When no handler is set then the sync operation is a no-op,
the result code is AVERROR(ENOSYS).
---
 libavformat/avio.c|  7 +++
 libavformat/avio.h| 20 
 libavformat/aviobuf.c | 17 +
 libavformat/url.h | 12 
 4 files changed, 56 insertions(+)

diff --git a/libavformat/avio.c b/libavformat/avio.c
index 663789ec02..5754a7c20d 100644
--- a/libavformat/avio.c
+++ b/libavformat/avio.c
@@ -623,6 +623,13 @@ int64_t ffurl_size(URLContext *h)
 return size;
 }
 
+int ffurl_sync(URLContext *h)
+{
+if (h->prot->url_sync)
+return h->prot->url_sync(h);
+return AVERROR(ENOSYS);
+}
+
 int ffurl_get_file_handle(URLContext *h)
 {
 if (!h || !h->prot || !h->prot->url_get_file_handle)
diff --git a/libavformat/avio.h b/libavformat/avio.h
index 75912ce6be..9e2ef14e60 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -349,6 +349,14 @@ typedef struct AVIOContext {
  * Try to buffer at least this amount of data before flushing it
  */
 int min_packet_size;
+
+/**
+ * A callback for synchronizing buffers with the media state.
+ *
+ * @return 0 on success, AVERROR(ENOSYS) if the operation is not supported
+ * and ignored, or other AVERROR < 0 on error.
+ */
+int (*sync)(void *opaque);
 } AVIOContext;
 
 /**
@@ -586,6 +594,18 @@ int avio_printf(AVIOContext *s, const char *fmt, ...) 
av_printf_format(2, 3);
  */
 void avio_flush(AVIOContext *s);
 
+/**
+ * Flush internal buffers and ensure the synchronized state of the
+ * resource associated with the context s. This call may be expensive.
+ * Not all resources support syncing, this operation is a no-op
+ * if sync is not supported or needed.
+ * This function can only be used if s was opened by avio_open().
+ *
+ * @return 0 on success, AVERROR(ENOSYS) if the operation is not supported
+ * and ignored, or other AVERROR < 0 on error.
+ */
+int avio_sync(AVIOContext *s);
+
 /**
  * Read size bytes from AVIOContext into buf.
  * @return number of bytes read or AVERROR
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 5a33f82950..c1c9334719 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -243,6 +243,14 @@ void avio_flush(AVIOContext *s)
 avio_seek(s, seekback, SEEK_CUR);
 }
 
+int avio_sync(AVIOContext *s)
+{
+avio_flush(s);
+if (s->sync)
+return s->sync(s->opaque);
+return AVERROR(ENOSYS);
+}
+
 int64_t avio_seek(AVIOContext *s, int64_t offset, int whence)
 {
 int64_t offset1;
@@ -978,6 +986,12 @@ static int64_t io_read_seek(void *opaque, int 
stream_index, int64_t timestamp, i
 return internal->h->prot->url_read_seek(internal->h, stream_index, 
timestamp, flags);
 }
 
+static int io_sync(void *opaque)
+{
+AVIOInternal *internal = opaque;
+return ffurl_sync(internal->h);
+}
+
 int ffio_fdopen(AVIOContext **s, URLContext *h)
 {
 AVIOInternal *internal = NULL;
@@ -1026,6 +1040,9 @@ int ffio_fdopen(AVIOContext **s, URLContext *h)
 
 if (h->prot->url_read_seek)
 (*s)->seekable |= AVIO_SEEKABLE_TIME;
+
+if (h->prot->url_sync)
+(*s)->sync = io_sync;
 }
 (*s)->short_seek_get = io_short_seek;
 (*s)->av_class = _avio_class;
diff --git a/libavformat/url.h b/libavformat/url.h
index 4750bfff82..5b8cd22e5a 100644
--- a/libavformat/url.h
+++ b/libavformat/url.h
@@ -97,6 +97,7 @@ typedef struct URLProtocol {
 int (*url_delete)(URLContext *h);
 int (*url_move)(URLContext *h_src, URLContext *h_dst);
 const char *default_whitelist;
+int (*url_sync)(URLContext *h);
 } URLProtocol;
 
 /**
@@ -228,6 +229,17 @@ int64_t ffurl_seek(URLContext *h, int64_t pos, int whence);
 int ffurl_closep(URLContext **h);
 int ffurl_close(URLContext *h);
 
+/**
+ * Flush any buffered data and synchronize the resource accessed
+ * by the URLContext h. This call may be expensive.
+ * Not all types of resources support syncing, the call is a no-op
+ * if sync is not supported or needed.
+ *
+ * @return 0 on success, AVERROR(ENOSYS) if the operation is not supported
+ * and ignored, or other AVERROR < 0 on error.
+ */
+int ffurl_sync(URLContext *h);
+
 /**
  * Return the filesize of the resource accessed by h, AVERROR(ENOSYS)
  * if the 

[FFmpeg-devel] [PATCH v3 2/2] lavf/file: Add support for file syncing.

[Andrey Semashev]

This commit adds support for IO synchronization API to the file backend.
---
 libavformat/file.c   | 11 +++
 libavformat/os_support.h |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/libavformat/file.c b/libavformat/file.c
index 1d321c4205..58fd55b928 100644
--- a/libavformat/file.c
+++ b/libavformat/file.c
@@ -254,6 +254,16 @@ static int64_t file_seek(URLContext *h, int64_t pos, int 
whence)
 return ret < 0 ? AVERROR(errno) : ret;
 }
 
+static int file_sync(URLContext *h)
+{
+if (h->flags & AVIO_FLAG_WRITE) {
+FileContext *c = h->priv_data;
+if (fsync(c->fd) < 0)
+return AVERROR(errno);
+}
+return 0;
+}
+
 static int file_close(URLContext *h)
 {
 FileContext *c = h->priv_data;
@@ -353,6 +363,7 @@ const URLProtocol ff_file_protocol = {
 .url_close   = file_close,
 .url_get_file_handle = file_get_handle,
 .url_check   = file_check,
+.url_sync= file_sync,
 .url_delete  = file_delete,
 .url_move= file_move,
 .priv_data_size  = sizeof(FileContext),
diff --git a/libavformat/os_support.h b/libavformat/os_support.h
index 7a56dc9a7c..1864763cb1 100644
--- a/libavformat/os_support.h
+++ b/libavformat/os_support.h
@@ -93,6 +93,8 @@ static inline int is_dos_path(const char *path)
 #ifndef S_IWUSR
 #define S_IWUSR S_IWRITE
 #endif
+
+#define fsync(fd) _commit((fd))
 #endif
 
 #if CONFIG_NETWORK
-- 
2.19.1

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

Re: [FFmpeg-devel] lavd: Remove libndi_newtek

[Andrey Semashev]

On 3/10/19 9:39 PM, Nicolas George wrote:

Andrey Semashev (12019-03-10):

But I think decisions like this should be based exclusively on
technical grounds. Political or populist arguments are not valid for
technical decisions, regardless how "good" or aligned with your personal
views they might seem.


Do you realize that this statement is itself a political one?


Yes. It doesn't make it less true, IMO.

Note that I'm not arguing for or against the removal. I don't really 
care. I'm pointing out that there seem to be no technical reason for the 
removal, but instead there is a political one. And apparently, there are 
users of the code out there, who will be affected.

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

Re: [FFmpeg-devel] lavd: Remove libndi_newtek

[Andrey Semashev]

On 3/10/19 8:28 PM, Jean-Baptiste Kempf wrote:

On Sun, 10 Mar 2019, at 18:04, Martin Vignali wrote:

This discussion just send bad message to contributors.
You're just encourage them to keep code improvement in their own version of
ffmpeg.


Maintaining an FFmpeg is hard and costly.

This gives those people incentives to use open source libraries or write new 
ones.


I would say, the message is quite the opposite.

I'm not an active member of the community, so please disregard my 
opinion if you will. But I think decisions like this should be based 
exclusively on technical grounds. Political or populist arguments are 
not valid for technical decisions, regardless how "good" or aligned with 
your personal views they might seem. That is, if you care about 
technical quality of your project and not just "fighting the good fight".

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

Re: [FFmpeg-devel] [PATCH 1/3] avutil/attributes: add av_likely and av_unlikely

[Andrey Semashev]

On 1/24/19 11:53 PM, Rostislav Pehlivanov wrote:

On Thu, 24 Jan 2019 at 20:38, Marton Balint  wrote:


Signed-off-by: Marton Balint 
---
  doc/APIchanges | 3 +++
  libavutil/attributes.h | 8 
  libavutil/version.h| 2 +-
  3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index a39a3ff2ba..38b5b980a6 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@ libavutil: 2017-10-21

  API changes, most recent first:

+2019-01-xx - xx - lavu 56.27.100 - attributes.h
+  Add av_likely and av_unlikely
+
  2019-01-08 - xx - lavu 56.26.100 - frame.h
Add AV_FRAME_DATA_REGIONS_OF_INTEREST

diff --git a/libavutil/attributes.h b/libavutil/attributes.h
index ced108aa2c..60972e5109 100644
--- a/libavutil/attributes.h
+++ b/libavutil/attributes.h
@@ -164,4 +164,12 @@
  #define av_noreturn
  #endif

+#if AV_GCC_VERSION_AT_LEAST(2,96) || defined(__clang__)
+# define av_likely(x)  __builtin_expect(!!(x), 1)
+# define av_unlikely(x)__builtin_expect(!!(x), 0)
+#else
+# define av_likely(x)  (x)
+# define av_unlikely(x)(x)
+#endif
+
  #endif /* AVUTIL_ATTRIBUTES_H */
diff --git a/libavutil/version.h b/libavutil/version.h
index 1fcdea95bf..12b4f9fc3a 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
   */

  #define LIBAVUTIL_VERSION_MAJOR  56
-#define LIBAVUTIL_VERSION_MINOR  26
+#define LIBAVUTIL_VERSION_MINOR  27
  #define LIBAVUTIL_VERSION_MICRO 100

  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
--
2.16.4

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




NAK, NAK, NAK.
I will NAK the hell out of any patch that does that. They're completely
unnecessary, they're commonly used by complete idiots who add them thinking
their code will go faster (and it might - only on their antiquated GCC
3.1), they're religiously used by the same people and won't back down on
using them and finally they're ugly when added to every single bloody
branch and the people who maintain such code will demand they be added to
every single bloody branch for no reason other that what I've just iterated
on.


There are valid reasons to want to use branch probability hints beyond 
aiding branch prediction. For example, reducing cache pollution in 
general and loop sizes in particular. Calling people who care about 
those things idiots and religiously denying the feature is an idiotic 
thing to do in its own right.

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

[FFmpeg-devel] [PATCH] configure: Update libmysofa check with a new symbol.

[Andrey Semashev]

The current code in libavfilter/af_sofalizer.c requires
mysofa_neighborhood_init_withstepdefine function, which only appeared
in libmysofa 0.7. Add this function to configure script to bail out
early if a too old libmysofa is found in the system instead of failing
at compile time.
---
 configure | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 51dc77c780..51fb8a7dd0 100755
--- a/configure
+++ b/configure
@@ -6237,8 +6237,8 @@ enabled libmfx&& { check_pkg_config libmfx 
libmfx "mfx/mfxvideo.h" M
{ require libmfx "mfx/mfxvideo.h" MFXInit 
"-llibmfx $advapi32_extralibs" && warn "using libmfx without pkg-config"; } }
 enabled libmodplug&& require_pkg_config libmodplug libmodplug 
libmodplug/modplug.h ModPlug_Load
 enabled libmp3lame&& require "libmp3lame >= 3.98.3" lame/lame.h 
lame_set_VBR_quality -lmp3lame $libm_extralibs
-enabled libmysofa && { check_pkg_config libmysofa libmysofa mysofa.h 
mysofa_load ||
-   require libmysofa mysofa.h mysofa_load -lmysofa 
$zlib_extralibs; }
+enabled libmysofa && { check_pkg_config libmysofa libmysofa mysofa.h 
"mysofa_load mysofa_neighborhood_init_withstepdefine" ||
+   require libmysofa mysofa.h "mysofa_load 
mysofa_neighborhood_init_withstepdefine" -lmysofa $zlib_extralibs; }
 enabled libnpp&& { check_lib libnpp npp.h nppGetLibVersion -lnppig 
-lnppicc -lnppc -lnppidei ||
check_lib libnpp npp.h nppGetLibVersion -lnppi 
-lnppc -lnppidei ||
die "ERROR: libnpp not found"; }
-- 
2.20.1

___
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] tests: Fix bash errors in lavf_container tests.

[Andrey Semashev]

Because lavf_container is sometimes called with only 2 arguments,
fate tests produce bash errors like this:

  tests/fate-run.sh: 299: test: =: unexpected operator

This commit fixes this.
---
 tests/fate-run.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/fate-run.sh b/tests/fate-run.sh
index 2f1991da52..aec12c16a3 100755
--- a/tests/fate-run.sh
+++ b/tests/fate-run.sh
@@ -296,7 +296,7 @@ lavf_container(){
 outdir="tests/data/lavf"
 file=${outdir}/lavf.$t
 do_avconv $file $DEC_OPTS -f image2 -c:v pgmyuv -i $raw_src $DEC_OPTS -ar 
44100 -f s16le $1 -i $pcm_src "$ENC_OPTS -metadata title=lavftest" -b:a 64k -t 
1 -qscale:v 10 $2
-test $3 = "disable_crc" ||
+test "$3" = "disable_crc" ||
 do_avconv_crc $file $DEC_OPTS -i $target_path/$file $3
 }
 
-- 
2.20.1

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

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

Re: [FFmpeg-devel] [PATCH] configure: Update libmysofa check with a new symbol.

[Andrey Semashev]

On 2019-08-28 18:41, James Almer wrote:

On 8/28/2019 12:24 PM, Andrey Semashev wrote:

The current code in libavfilter/af_sofalizer.c requires
mysofa_neighborhood_init_withstepdefine function, which only appeared
in libmysofa 0.7. Add this function to configure script to bail out
early if a too old libmysofa is found in the system instead of failing
at compile time.
---
  configure | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 51dc77c780..51fb8a7dd0 100755
--- a/configure
+++ b/configure
@@ -6237,8 +6237,8 @@ enabled libmfx&& { check_pkg_config libmfx libmfx 
"mfx/mfxvideo.h" M
 { require libmfx "mfx/mfxvideo.h" MFXInit "-llibmfx 
$advapi32_extralibs" && warn "using libmfx without pkg-config"; } }
  enabled libmodplug&& require_pkg_config libmodplug libmodplug 
libmodplug/modplug.h ModPlug_Load
  enabled libmp3lame&& require "libmp3lame >= 3.98.3" lame/lame.h 
lame_set_VBR_quality -lmp3lame $libm_extralibs
-enabled libmysofa && { check_pkg_config libmysofa libmysofa mysofa.h 
mysofa_load ||
-   require libmysofa mysofa.h mysofa_load -lmysofa 
$zlib_extralibs; }
+enabled libmysofa && { check_pkg_config libmysofa libmysofa mysofa.h 
"mysofa_load mysofa_neighborhood_init_withstepdefine" ||
+   require libmysofa mysofa.h "mysofa_load 
mysofa_neighborhood_init_withstepdefine" -lmysofa $zlib_extralibs; }


You could instead make the pkg-config check for "libmysofa >= 0.7",
assuming the project generated .pc file reports correct versions.


I don't know exactly how the library version will be represented in the 
.pc file (e.g. it may include a revision or some such). There is no .pc 
file for libmysofa in Ubuntu, which is where I'm working. Someone who 
can test the .pc file can add the version check later.



  enabled libnpp&& { check_lib libnpp npp.h nppGetLibVersion 
-lnppig -lnppicc -lnppc -lnppidei ||
 check_lib libnpp npp.h nppGetLibVersion -lnppi 
-lnppc -lnppidei ||
 die "ERROR: libnpp not found"; }



___
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".

Re: [FFmpeg-devel] [PATCH] tests: Fix bash errors in lavf_container tests.

[Andrey Semashev]

Ping?

On 2019-08-28 18:32, Andrey Semashev wrote:

Because lavf_container is sometimes called with only 2 arguments,
fate tests produce bash errors like this:

   tests/fate-run.sh: 299: test: =: unexpected operator

This commit fixes this.
---
  tests/fate-run.sh | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/fate-run.sh b/tests/fate-run.sh
index 2f1991da52..aec12c16a3 100755
--- a/tests/fate-run.sh
+++ b/tests/fate-run.sh
@@ -296,7 +296,7 @@ lavf_container(){
  outdir="tests/data/lavf"
  file=${outdir}/lavf.$t
  do_avconv $file $DEC_OPTS -f image2 -c:v pgmyuv -i $raw_src $DEC_OPTS -ar 44100 -f 
s16le $1 -i $pcm_src "$ENC_OPTS -metadata title=lavftest" -b:a 64k -t 1 
-qscale:v 10 $2
-test $3 = "disable_crc" ||
+test "$3" = "disable_crc" ||
  do_avconv_crc $file $DEC_OPTS -i $target_path/$file $3
  }
  



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

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

Re: [FFmpeg-devel] [PATCH v2] configure: Update libmysofa check with a new symbol.

[Andrey Semashev]

Ping? I would like this to make it into 4.2 as well.

On Wed, Aug 28, 2019 at 11:16 PM Andrey Semashev
 wrote:
>
> The current code in libavfilter/af_sofalizer.c requires
> mysofa_neighborhood_init_withstepdefine function, which only appeared
> in libmysofa 0.7. Use this function in configure script to bail out
> early if a too old libmysofa is found in the system instead of failing
> at compile time.
> ---
>  configure | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index 51dc77c780..0d06ea6a02 100755
> --- a/configure
> +++ b/configure
> @@ -6237,8 +6237,8 @@ enabled libmfx&& { check_pkg_config libmfx 
> libmfx "mfx/mfxvideo.h" M
> { require libmfx "mfx/mfxvideo.h" MFXInit 
> "-llibmfx $advapi32_extralibs" && warn "using libmfx without pkg-config"; } }
>  enabled libmodplug&& require_pkg_config libmodplug libmodplug 
> libmodplug/modplug.h ModPlug_Load
>  enabled libmp3lame&& require "libmp3lame >= 3.98.3" lame/lame.h 
> lame_set_VBR_quality -lmp3lame $libm_extralibs
> -enabled libmysofa && { check_pkg_config libmysofa libmysofa mysofa.h 
> mysofa_load ||
> -   require libmysofa mysofa.h mysofa_load 
> -lmysofa $zlib_extralibs; }
> +enabled libmysofa && { check_pkg_config libmysofa libmysofa mysofa.h 
> mysofa_neighborhood_init_withstepdefine ||
> +   require libmysofa mysofa.h 
> mysofa_neighborhood_init_withstepdefine -lmysofa $zlib_extralibs; }
>  enabled libnpp&& { check_lib libnpp npp.h nppGetLibVersion 
> -lnppig -lnppicc -lnppc -lnppidei ||
> check_lib libnpp npp.h nppGetLibVersion 
> -lnppi -lnppc -lnppidei ||
> die "ERROR: libnpp not found"; }
> --
> 2.20.1
>
___
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 v2] configure: Update libmysofa check with a new symbol.

[Andrey Semashev]

The current code in libavfilter/af_sofalizer.c requires
mysofa_neighborhood_init_withstepdefine function, which only appeared
in libmysofa 0.7. Use this function in configure script to bail out
early if a too old libmysofa is found in the system instead of failing
at compile time.
---
 configure | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 51dc77c780..0d06ea6a02 100755
--- a/configure
+++ b/configure
@@ -6237,8 +6237,8 @@ enabled libmfx&& { check_pkg_config libmfx 
libmfx "mfx/mfxvideo.h" M
{ require libmfx "mfx/mfxvideo.h" MFXInit 
"-llibmfx $advapi32_extralibs" && warn "using libmfx without pkg-config"; } }
 enabled libmodplug&& require_pkg_config libmodplug libmodplug 
libmodplug/modplug.h ModPlug_Load
 enabled libmp3lame&& require "libmp3lame >= 3.98.3" lame/lame.h 
lame_set_VBR_quality -lmp3lame $libm_extralibs
-enabled libmysofa && { check_pkg_config libmysofa libmysofa mysofa.h 
mysofa_load ||
-   require libmysofa mysofa.h mysofa_load -lmysofa 
$zlib_extralibs; }
+enabled libmysofa && { check_pkg_config libmysofa libmysofa mysofa.h 
mysofa_neighborhood_init_withstepdefine ||
+   require libmysofa mysofa.h 
mysofa_neighborhood_init_withstepdefine -lmysofa $zlib_extralibs; }
 enabled libnpp&& { check_lib libnpp npp.h nppGetLibVersion -lnppig 
-lnppicc -lnppc -lnppidei ||
check_lib libnpp npp.h nppGetLibVersion -lnppi 
-lnppc -lnppidei ||
die "ERROR: libnpp not found"; }
-- 
2.20.1

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

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

Re: [FFmpeg-devel] [PATCH] configure: Update libmysofa check with a new symbol.

[Andrey Semashev]

On 2019-08-28 21:34, Carl Eugen Hoyos wrote:

Am Mi., 28. Aug. 2019 um 17:33 Uhr schrieb Andrey Semashev
:


The current code in libavfilter/af_sofalizer.c requires
mysofa_neighborhood_init_withstepdefine function, which only appeared
in libmysofa 0.7. Add this function to configure script to bail out
early if a too old libmysofa is found in the system instead of failing
at compile time.
---
  configure | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 51dc77c780..51fb8a7dd0 100755
--- a/configure
+++ b/configure
@@ -6237,8 +6237,8 @@ enabled libmfx&& { check_pkg_config libmfx libmfx 
"mfx/mfxvideo.h" M
 { require libmfx "mfx/mfxvideo.h" MFXInit "-llibmfx 
$advapi32_extralibs" && warn "using libmfx without pkg-config"; } }
  enabled libmodplug&& require_pkg_config libmodplug libmodplug 
libmodplug/modplug.h ModPlug_Load
  enabled libmp3lame&& require "libmp3lame >= 3.98.3" lame/lame.h 
lame_set_VBR_quality -lmp3lame $libm_extralibs
-enabled libmysofa && { check_pkg_config libmysofa libmysofa mysofa.h 
mysofa_load ||
-   require libmysofa mysofa.h mysofa_load -lmysofa 
$zlib_extralibs; }
+enabled libmysofa && { check_pkg_config libmysofa libmysofa mysofa.h 
"mysofa_load mysofa_neighborhood_init_withstepdefine" ||
+   require libmysofa mysofa.h "mysofa_load 
mysofa_neighborhood_init_withstepdefine" -lmysofa $zlib_extralibs; }


Can you remove mysofa_load?


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

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

Re: [FFmpeg-devel] [Compiled Failed] Trying to compile FFmpeg with mysofa and it failed at af_sofalizer.c:164:32

[Andrey Semashev]

On 12/29/18 10:15 PM, James Almer wrote:

On 12/29/2018 8:09 AM, Paul B Mahol wrote:

On 12/29/18, Ysy  wrote:

I was trying to compile FFmpeg with `--enable-libmysofa`, and it failed with
this message below:

src/libavfilter/af_sofalizer.c:164:32: error: implicit declaration of
function 'mysofa_neighborhood_init_withstepdefine' is invalid in C99
   [-Werror,-Wimplicit-function-declaration]
 s->sofa.neighborhood =
mysofa_neighborhood_init_withstepdefine(s->sofa.hrtf,
^
src/libavfilter/af_sofalizer.c:164:32: warning: this function declaration is
not a prototype [-Wstrict-prototypes]
src/libavfilter/af_sofalizer.c:164:30: warning: incompatible integer to
pointer conversion assigning to 'struct MYSOFA_NEIGHBORHOOD *' from 'int'
   [-Wint-conversion]
 s->sofa.neighborhood =
mysofa_neighborhood_init_withstepdefine(s->sofa.hrtf,
  ^
~
2 warnings and 1 error generated.

Device:
macOS Mojave (10.14.2)
Apple LLVM version 10.0.0 (clang-1000.11.45.5)
Target: x86_64-apple-darwin18.2.0
Thread model: posix

The code was committed 6 days ago with comment `stop using easy API` in
master branch.

I’ve noticed that this function (`mysofa_neighborhood_init_withstepdefine `)
is really not yet exist on this project. Maybe it is not completed yet?

Thank you for watching this.


Update libmysofa lib.


What's the first version that works? The configure check should be
updated to bail out if it's too old.


Is there a resolution? What is the min version of libmysofa?

I'm failing to compile ffmpeg 4.2 on Ubuntu 19.04 because of this.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] tests: Fix bash errors in lavf_container tests.

[Andrey Semashev]

Could someone take a look at this small fix, please? I'd like it to make 
it into 4.2.1.


On 2019-09-02 13:54, Andrey Semashev wrote:

Ping?

On 2019-08-28 18:32, Andrey Semashev wrote:

Because lavf_container is sometimes called with only 2 arguments,
fate tests produce bash errors like this:

   tests/fate-run.sh: 299: test: =: unexpected operator

This commit fixes this.
---
  tests/fate-run.sh | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/fate-run.sh b/tests/fate-run.sh
index 2f1991da52..aec12c16a3 100755
--- a/tests/fate-run.sh
+++ b/tests/fate-run.sh
@@ -296,7 +296,7 @@ lavf_container(){
  outdir="tests/data/lavf"
  file=${outdir}/lavf.$t
  do_avconv $file $DEC_OPTS -f image2 -c:v pgmyuv -i $raw_src 
$DEC_OPTS -ar 44100 -f s16le $1 -i $pcm_src "$ENC_OPTS -metadata 
title=lavftest" -b:a 64k -t 1 -qscale:v 10 $2

-    test $3 = "disable_crc" ||
+    test "$3" = "disable_crc" ||
  do_avconv_crc $file $DEC_OPTS -i $target_path/$file $3
  }





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

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

Re: [FFmpeg-devel] Is it ok to add G.722.1 decoder as external lib?

[Andrey Semashev]

On 2019-09-17 03:29, Hyun Yoo wrote:

I implemented a g.722.1 decoder by linking FreeSwitch's libg722_1
as external lib like libilbc, libspeex(ex. configure --enable-libg722_1)
(https://github.com/traviscross/freeswitch/tree/master/libs/libg722_1)


I believe, the correct upstream link is

https://freeswitch.org/stash/projects/SD/repos/libg7221/browse

The GitHub project is a mirror.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] tests: Fix bash errors in lavf_container tests.

[Andrey Semashev]

On 2019-09-07 18:32, Limin Wang wrote:

On Sat, Sep 07, 2019 at 05:19:55PM +0200, Michael Niedermayer wrote:

On Wed, Aug 28, 2019 at 06:32:37PM +0300, Andrey Semashev wrote:

Because lavf_container is sometimes called with only 2 arguments,
fate tests produce bash errors like this:

   tests/fate-run.sh: 299: test: =: unexpected operator

This commit fixes this.
---
  tests/fate-run.sh | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


I think this change is correct but shell is not my area ...

If the $2 is two arguments between with space, the patch is needed.


$2 is a single argument that may contain spaces. It is interpreted as a 
single argument as long as it is enclosed in quotes in lavf_container 
invokation.


The problem is caused not by $2 but by the fact there is no $3. See 
lavf_container_attach, lavf_container_timecode_nodrop, 
lavf_container_timecode_drop and lavf_container_timecode functions, as 
well as a few tests in tests/fate/lavf-container.mak.



Why I haven't got such errors when I'm running fate?


I noticed the errors when the tests failed (due to my local changes not 
relevant to this patch). I don't know the details about how tests are 
run, but maybe the output is suppressed when the tests pass?

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

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

Re: [FFmpeg-devel] [PATCH v2] configure: Update libmysofa check with a new symbol.

[Andrey Semashev]

On 2019-09-02 20:10, Paul B Mahol wrote:

On 9/2/19, Andrey Semashev  wrote:

Ping? I would like this to make it into 4.2 as well.



Applied to master.


Thanks. Could you also merge it to 4.2 branch?


On Wed, Aug 28, 2019 at 11:16 PM Andrey Semashev
 wrote:


The current code in libavfilter/af_sofalizer.c requires
mysofa_neighborhood_init_withstepdefine function, which only appeared
in libmysofa 0.7. Use this function in configure script to bail out
early if a too old libmysofa is found in the system instead of failing
at compile time.
---
  configure | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 51dc77c780..0d06ea6a02 100755
--- a/configure
+++ b/configure
@@ -6237,8 +6237,8 @@ enabled libmfx&& { check_pkg_config
libmfx libmfx "mfx/mfxvideo.h" M
 { require libmfx "mfx/mfxvideo.h" MFXInit
"-llibmfx $advapi32_extralibs" && warn "using libmfx without pkg-config";
} }
  enabled libmodplug&& require_pkg_config libmodplug libmodplug
libmodplug/modplug.h ModPlug_Load
  enabled libmp3lame&& require "libmp3lame >= 3.98.3" lame/lame.h
lame_set_VBR_quality -lmp3lame $libm_extralibs
-enabled libmysofa && { check_pkg_config libmysofa libmysofa
mysofa.h mysofa_load ||
-   require libmysofa mysofa.h mysofa_load
-lmysofa $zlib_extralibs; }
+enabled libmysofa && { check_pkg_config libmysofa libmysofa
mysofa.h mysofa_neighborhood_init_withstepdefine ||
+   require libmysofa mysofa.h
mysofa_neighborhood_init_withstepdefine -lmysofa $zlib_extralibs; }
  enabled libnpp&& { check_lib libnpp npp.h nppGetLibVersion
-lnppig -lnppicc -lnppc -lnppidei ||
 check_lib libnpp npp.h nppGetLibVersion
-lnppi -lnppc -lnppidei ||
 die "ERROR: libnpp not found"; }
--
2.20.1


___
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 mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Extract QP from h264 encoded videos

[Andrey Semashev]

On 7/30/19 12:11 PM, Andrey Semashev wrote:

On 7/30/19 5:39 AM, Juan De León wrote:
I tried to fix all you suggested, please have a look and let me know 
what you think.


design doc: 
https://docs.google.com/document/d/1WClt3EqhjwdGXhEw386O0wfn3IBQ1Ib-_5emVM1gbnA/edit?usp=sharing 



Signed-off-by: Juan De León 
---
  libavutil/Makefile  |   2 +
  libavutil/frame.h   |   6 ++
  libavutil/quantization_params.c |  42 
  libavutil/quantization_params.h | 114 
  4 files changed, 164 insertions(+)
  create mode 100644 libavutil/quantization_params.c
  create mode 100644 libavutil/quantization_params.h

diff --git a/libavutil/Makefile b/libavutil/Makefile
index 8a7a44e4b5..be1a9c3a9c 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -60,6 +60,7 @@ HEADERS = 
adler32.h \

pixdesc.h \

pixelutils.h  \

pixfmt.h  \
+  
quantization_params.h \

random_seed.h \

rc4.h \

rational.h    \
@@ -140,6 +141,7 @@ OBJS = 
adler32.o    \
 
parseutils.o \
 
pixdesc.o    \
 
pixelutils.o \
+   
quantization_params.o    \
 
random_seed.o    \
 
rational.o   \
 
reverse.o    \

diff --git a/libavutil/frame.h b/libavutil/frame.h
index 5d3231e7bb..b64fd9c02c 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -179,6 +179,12 @@ enum AVFrameSideDataType {
   * array element is implied by AVFrameSideData.size / 
AVRegionOfInterest.self_size.

   */
  AV_FRAME_DATA_REGIONS_OF_INTEREST,
+    /**
+ * To extract quantization parameters from supported decoders.
+ * The data is stored as AVQuantizationParamsArray type, 
described in

+ * libavuitl/quantization_params.h
+ */
+    AV_FRAME_DATA_QUANTIZATION_PARAMS,
  };
  enum AVActiveFormatDescription {
diff --git a/libavutil/quantization_params.c 
b/libavutil/quantization_params.c

new file mode 100644
index 00..fc51b55eee
--- /dev/null
+++ b/libavutil/quantization_params.c
@@ -0,0 +1,42 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 
02110-1301 USA

+ */
+
+#include "libavutil/quantization_params.h"
+#include "libavutil/mem.h"
+
+static const char* const QP_NAMES_H264[] = {"qpy", "qpuv"};
+
+static const char* const QP_NAMES_VP9[] = {"qyac", "qydc", "quvdc", 
"quvac",
+   "qiyac", "qiydc", 
"qiuvdc", "qiuvac"};

+
+static const char* const QP_NAMES_AV1[] = {"qydc", "qyac", "qudc", 
"quac", "qvdc", "qvac",
+  "qiydc", "qiyac", "qiudc", 
"qiuac", "qivdc", "qivac"};

+
+char* av_get_qp_type_string(enum AVExtractQPSupportedCodecs codec_id, 
int index)

+{
+    switch (codec_id) {
+    case AV_EXTRACT_QP_CODEC_ID_H264:
+    return index < AV_QP_ARR_SIZE_H264 ? 
av_strdup(QP_NAMES_H264[index]) :NULL;

+    case AV_EXTRACT_QP_CODEC_ID_VP9:
+    return index < AV_QP_ARR_SIZE_VP9  ? 
av_strdup(QP_NAMES_VP9[index])  :NULL;

+    case AV_EXTRACT_QP_CODEC_ID_AV1:
+    return index < AV_QP_ARR_SIZE_AV1  ? 
av_strdup(QP_NAMES_AV1[index])  :NULL;

+    default:
+    return NULL;
+    }
+}


Why strdup he

Re: [FFmpeg-devel] [PATCH] Extract QP from h264 encoded videos

[Andrey Semashev]

On 7/30/19 5:39 AM, Juan De León wrote:

I tried to fix all you suggested, please have a look and let me know what you 
think.

design doc: 
https://docs.google.com/document/d/1WClt3EqhjwdGXhEw386O0wfn3IBQ1Ib-_5emVM1gbnA/edit?usp=sharing

Signed-off-by: Juan De León 
---
  libavutil/Makefile  |   2 +
  libavutil/frame.h   |   6 ++
  libavutil/quantization_params.c |  42 
  libavutil/quantization_params.h | 114 
  4 files changed, 164 insertions(+)
  create mode 100644 libavutil/quantization_params.c
  create mode 100644 libavutil/quantization_params.h

diff --git a/libavutil/Makefile b/libavutil/Makefile
index 8a7a44e4b5..be1a9c3a9c 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -60,6 +60,7 @@ HEADERS = adler32.h   
  \
pixdesc.h \
pixelutils.h  \
pixfmt.h  \
+  quantization_params.h \
random_seed.h \
rc4.h \
rational.h\
@@ -140,6 +141,7 @@ OBJS = adler32.o
\
 parseutils.o \
 pixdesc.o\
 pixelutils.o \
+   quantization_params.o\
 random_seed.o\
 rational.o   \
 reverse.o\
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 5d3231e7bb..b64fd9c02c 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -179,6 +179,12 @@ enum AVFrameSideDataType {
   * array element is implied by AVFrameSideData.size / 
AVRegionOfInterest.self_size.
   */
  AV_FRAME_DATA_REGIONS_OF_INTEREST,
+/**
+ * To extract quantization parameters from supported decoders.
+ * The data is stored as AVQuantizationParamsArray type, described in
+ * libavuitl/quantization_params.h
+ */
+AV_FRAME_DATA_QUANTIZATION_PARAMS,
  };
  
  enum AVActiveFormatDescription {

diff --git a/libavutil/quantization_params.c b/libavutil/quantization_params.c
new file mode 100644
index 00..fc51b55eee
--- /dev/null
+++ b/libavutil/quantization_params.c
@@ -0,0 +1,42 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/quantization_params.h"
+#include "libavutil/mem.h"
+
+static const char* const QP_NAMES_H264[] = {"qpy", "qpuv"};
+
+static const char* const QP_NAMES_VP9[] = {"qyac", "qydc", "quvdc", "quvac",
+   "qiyac", "qiydc", "qiuvdc", 
"qiuvac"};
+
+static const char* const QP_NAMES_AV1[] = {"qydc", "qyac", "qudc", "quac", "qvdc", 
"qvac",
+  "qiydc", "qiyac", "qiudc", "qiuac", "qivdc", 
"qivac"};
+
+char* av_get_qp_type_string(enum AVExtractQPSupportedCodecs codec_id, int 
index)
+{
+switch (codec_id) {
+case AV_EXTRACT_QP_CODEC_ID_H264:
+return index < AV_QP_ARR_SIZE_H264 ? 
av_strdup(QP_NAMES_H264[index]) :NULL;
+case AV_EXTRACT_QP_CODEC_ID_VP9:
+return index < AV_QP_ARR_SIZE_VP9  ? 
av_strdup(QP_NAMES_VP9[index])  :NULL;
+case AV_EXTRACT_QP_CODEC_ID_AV1:
+return index < AV_QP_ARR_SIZE_AV1  ? 
av_strdup(QP_NAMES_AV1[index])  :NULL;
+default:
+return NULL;
+}
+}
diff --git a/libavutil/quantization_params.h b/libavutil/quantization_params.h
new file mode 100644
index 00..d123aade3b
--- /dev/null
+++ b/libavutil/quantization_params.h
@@ -0,0 +1,114 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the 

Re: [FFmpeg-devel] [PATCH] Extract QP from h264 encoded videos

[Andrey Semashev]

Just a few random comments. Disclaimer: I'm not a maintainer.

On 7/29/19 9:09 PM, Juan De León wrote:

Changes to libavcodec, hope this addresses all your comments.
Cheers.

Signed-off-by: Juan De León 
---
  libavutil/Makefile  |   2 +
  libavutil/frame.h   |   6 ++
  libavutil/quantization_params.c |  41 
  libavutil/quantization_params.h | 106 
  4 files changed, 155 insertions(+)
  create mode 100644 libavutil/quantization_params.c
  create mode 100644 libavutil/quantization_params.h

diff --git a/libavutil/Makefile b/libavutil/Makefile
index 8a7a44e4b5..be5e5d831f 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -60,6 +60,7 @@ HEADERS = adler32.h   
  \
pixdesc.h \
pixelutils.h  \
pixfmt.h  \
+  quantization_params.o
\


.h?


random_seed.h \
rc4.h \
rational.h\
@@ -140,6 +141,7 @@ OBJS = adler32.o
\
 parseutils.o \
 pixdesc.o\
 pixelutils.o \
+   quantization_params.o\
 random_seed.o\
 rational.o   \
 reverse.o\
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 5d3231e7bb..d48ccf342f 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -179,6 +179,12 @@ enum AVFrameSideDataType {
   * array element is implied by AVFrameSideData.size / 
AVRegionOfInterest.self_size.
   */
  AV_FRAME_DATA_REGIONS_OF_INTEREST,
+/**
+ * To extract quantization parameters from supported decoders.
+ * The data stored is AVQuantizationParamsArray type, described in


The data is stored as AVQuantizationParamsArray, described...


+ * libavuitls/quantization_params.h


libavutil


+ */
+AV_FRAME_DATA_QUANTIZATION_PARAMS,
  };
  
  enum AVActiveFormatDescription {

diff --git a/libavutil/quantization_params.c b/libavutil/quantization_params.c
new file mode 100644
index 00..28b08ebe19
--- /dev/null
+++ b/libavutil/quantization_params.c
@@ -0,0 +1,41 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/quantization_params.h"
+
+static const char* const QP_NAMES_H264[] = {"qp"};
+
+static const char* const QP_NAMES_VP9[] = {"qydc", "qyac", "quvdc", "quvac",
+   "qiydc", "qiyac", "qiuvdc", 
"qiuvac"};
+
+static const char* const QP_NAMES_AV1[] = {"qydc", "qyac", "qudc", "quac", "qvdc", 
"qvac",
+  "qiydc", "qiyac", "qiudc", "qiuac", "qivdc", 
"qivac"};
+
+char* get_qp_str(enum AVCodecID codec_id, int index)
+{
+switch (codec_id) {
+case AV_CODEC_ID_H264:
+return QP_NAMES_H264[index];
+case AV_CODEC_ID_VP9:
+return QP_NAMES_VP9[index];
+case AV_CODEC_ID_AV1:
+return QP_NAMES_AV1[index];
+default:
+return NULL;
+}
+}
diff --git a/libavutil/quantization_params.h b/libavutil/quantization_params.h
new file mode 100644
index 00..7a3daeaae5
--- /dev/null
+++ b/libavutil/quantization_params.h
@@ -0,0 +1,106 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is 

Re: [FFmpeg-devel] [PATCH] libavcodec/h261dec: Fix keyframe markup and frame skipping.

[Andrey Semashev]

On 2019-10-30 14:40, Michael Niedermayer wrote:

On Tue, Oct 29, 2019 at 04:39:16PM +0300, Andrey Semashev wrote:

On 2019-10-26 14:05, Andrey Semashev wrote:

The decoder never marks pictures as I-frames, which results in no
keyframe indication and incorrect frame skipping, in cases when
keyframes should be decoded.

This commit works around this decoder limitation and marks I-frames
and keyframes based on "freeze picture release" bit in h261 picture
header. This reflects h261enc behavior.


So, is this patch acceptable? If yes, could someone merge it please?


The patch does 2 things

1. it skips !freeze_picture_release frames if the user wants non key frames
 skiped

that seems reasonable


2. it marks freeze_picture_release frames as key/intra frames

i do not know if this is a good idea as i do not know if every
encoder sets this on key frames. Our encoder is not every encoder

If the patch intstead would set the flag depending on the blocks
then i would apply it

otherwise it would be nice to see a bit more evidence that this
flag is really always set for keyframes by most encoders.
Or maybe someone "knows" this then it can be applied too.

So I cant awnser if the patch is acceptable as is as i do not
know from the information provided how widely this flag is set
on keyframes


Obviously, I cannot say that every encoder in the world sets 
freeze_picture_release on keyframes. I quoted the H.261 spec that says 
that that should be the case and h261enc follows it. I cannot provide 
any more solid evidence.


PS: Note that skipping non-keyframes and marking frames as keyframes 
should be related. You can't accept the skipping part but not the 
marking part.

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

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

Re: [FFmpeg-devel] [PATCH] [libdav1d.c]: Add option to output all the spatial layers.

[Andrey Semashev]

On 2019-11-14 20:19, James Almer wrote:

On 11/14/2019 2:15 PM, Thierry Foucu wrote:

Set the option to false by default, to match libaomdec wrapper.
---
  libavcodec/libdav1d.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index cf4b178f1d..5efa8eeb48 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -40,6 +40,7 @@ typedef struct Libdav1dContext {
  int tile_threads;
  int frame_threads;
  int apply_grain;
+int all_layers;
  } Libdav1dContext;
  
  static const enum AVPixelFormat pix_fmt[][3] = {

@@ -134,6 +135,8 @@ static av_cold int libdav1d_init(AVCodecContext *c)
  if (dav1d->apply_grain >= 0)
  s.apply_grain = dav1d->apply_grain;
  
+s.all_layers = dav1d->all_layers;

+
  s.n_tile_threads = dav1d->tile_threads
   ? dav1d->tile_threads
   : FFMIN(floor(sqrt(threads)), DAV1D_MAX_TILE_THREADS);
@@ -378,6 +381,7 @@ static const AVOption libdav1d_options[] = {
  { "tilethreads", "Tile threads", OFFSET(tile_threads), AV_OPT_TYPE_INT, { 
.i64 = 0 }, 0, DAV1D_MAX_TILE_THREADS, VD },
  { "framethreads", "Frame threads", OFFSET(frame_threads), 
AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_FRAME_THREADS, VD },
  { "filmgrain", "Apply Film Grain", OFFSET(apply_grain), AV_OPT_TYPE_BOOL, 
{ .i64 = -1 }, -1, 1, VD },
+{ "alllayers", "Output all spatial layers", OFFSET(all_layers), 
AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD },
  { NULL }
  };


IMO, we don't want to output all layers under any circumstance. It'll
result in a mix of frames with duplicate pts and different dimensions.


IIRC, for VP9 with spatial SVC lavc decoder worked exactly like that - 
by returning a frame per spatial layer. Is this considered a bug?

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

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

Re: [FFmpeg-devel] [PATCH] [libdav1d.c]: Add option to output all the spatial layers.

[Andrey Semashev]

On 2019-11-14 20:48, James Almer wrote:

On 11/14/2019 2:31 PM, Andrey Semashev wrote:

On 2019-11-14 20:19, James Almer wrote:

On 11/14/2019 2:15 PM, Thierry Foucu wrote:

Set the option to false by default, to match libaomdec wrapper.
---
   libavcodec/libdav1d.c | 4 
   1 file changed, 4 insertions(+)

diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index cf4b178f1d..5efa8eeb48 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -40,6 +40,7 @@ typedef struct Libdav1dContext {
   int tile_threads;
   int frame_threads;
   int apply_grain;
+    int all_layers;
   } Libdav1dContext;
     static const enum AVPixelFormat pix_fmt[][3] = {
@@ -134,6 +135,8 @@ static av_cold int libdav1d_init(AVCodecContext *c)
   if (dav1d->apply_grain >= 0)
   s.apply_grain = dav1d->apply_grain;
   +    s.all_layers = dav1d->all_layers;
+
   s.n_tile_threads = dav1d->tile_threads
    ? dav1d->tile_threads
    : FFMIN(floor(sqrt(threads)),
DAV1D_MAX_TILE_THREADS);
@@ -378,6 +381,7 @@ static const AVOption libdav1d_options[] = {
   { "tilethreads", "Tile threads", OFFSET(tile_threads),
AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_TILE_THREADS, VD },
   { "framethreads", "Frame threads", OFFSET(frame_threads),
AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_FRAME_THREADS, VD },
   { "filmgrain", "Apply Film Grain", OFFSET(apply_grain),
AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, VD },
+    { "alllayers", "Output all spatial layers", OFFSET(all_layers),
AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD },
   { NULL }
   };


IMO, we don't want to output all layers under any circumstance. It'll
result in a mix of frames with duplicate pts and different dimensions.


IIRC, for VP9 with spatial SVC lavc decoder worked exactly like that -
by returning a frame per spatial layer. Is this considered a bug?


I don't know if it's a bug, and never seen or tested vp9 svc samples
before. Ronald might know. But it sounds like having more than one video
"stream" per AVCodecContext, which i don't think was intended.


There is one stream, in which an encoded frame is a VP9 superframe that 
contains frames for each spatial layer. From the user's perspective, VP9 
superframe is one entity.



And if it was, then it would be up to the library user to figure what to
do with these frames.


I think there needs to be some consistency across different lavc 
decoders. If we consider that lavc should produce one decoded frame per 
one encoded one, even if the encoded one contains multiple layers, then 
that should be true for all decoders.


Also, I think having decoded frames from all layers would also be 
useful, but there should be a way to know which layer they belong to. 
AFAIK, lavc currently doesn't provide that information. This mode of 
operation (producing frames for all layers) should be optional.

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

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

Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque

[Andrey Semashev]

On 2019-10-18 02:16, James Almer wrote:

On 10/17/2019 7:46 PM, Andrey Semashev wrote:

On 2019-10-18 01:28, James Almer wrote:

On 10/17/2019 7:13 PM, Andrey Semashev wrote:

On 2019-10-17 23:11, James Almer wrote:

Actually reorder the values.

Should effectively fix ticket #8300.

Signed-off-by: James Almer 
---
    libavcodec/libdav1d.c | 21 -
    1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext
*c, AVFrame *frame)
      pkt.buf = NULL;
    av_packet_unref();
+
+    if (c->reordered_opaque != AV_NOPTS_VALUE) {


I'm not sure this comparison is valid. The default value of
reordered_opaque is 0, and there seems to be no convention whatsoever
about what this value represents (i.e. its semantics are completely
user-defined). I think, this check needs to be removed and the code
below should execute for any reordered_opaque values.


AVCodecContext->reordered_opaque is by default AV_NOPTS_VALUE, as set in
avcodec_alloc_context3(). This field is normally used for timestamps
(despite not being the only use), and 0 is a valid timestamp, so that
can't be considered a "not set" value.


Ok, I see. I was looking at AVFrame initialization, which sets it to 0
by default.


And I'd rather not make this code unconditional. It's an allocation per
frame in a zero copy optimized decoder, and i don't want that overhead
when reordered_opaque is rarely going to be used.

Fwiw, timestamps are properly reordered by libdav1d in this same
function and propagated in the output frame. reordered_opaque is not
really needed for them.


FWIW, I'm the reporter of #8300 and our main reason for using
reordered_opaque instead of pts is that we don't want to mess with
timestamp conversion between our time base and whatever time_base
libavcodec might select for a given codec. So, we use reordered_opaque
universally, and it just happened to break with libdav1d.

Testing for AV_NOPTS_VALUE is ok in our particular case, though I had
the impression that ffmpeg is not supposed to interpret
reordered_opaque, including not assume it contains a timestamp.


Unfortunately you're right, and the check is probably not proper use of
the field, even if valid for pretty much any normal use case for it.

Will remove it, and simply deal with the malloc overhead.


You could avoid malloc completely on 64-bit systems by passing 
reordered_opaque inside the pointer to user data. It's not pretty, but 
it would get the job done. 32-bit systems would still have to malloc, 
unfortunately.

___
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 v3] avcodec/libdav1d: fix setting AVFrame reordered_opaque

[Andrey Semashev]

Actually reorder the values.

Should effectively fix ticket #8300.

Signed-off-by: James Almer 

Updated to avoid extra memory allocations on 64-bit platforms.

Signed-off-by: Andrey Semashev 
---
 libavcodec/libdav1d.c | 46 ++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..ff94310b40 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -19,12 +19,14 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include 
 #include 
 
 #include "libavutil/avassert.h"
 #include "libavutil/mastering_display_metadata.h"
 #include "libavutil/imgutils.h"
 #include "libavutil/opt.h"
+#include "libavutil/mem.h"
 
 #include "avcodec.h"
 #include "decode.h"
@@ -164,6 +166,12 @@ static void libdav1d_data_free(const uint8_t *data, void 
*opaque) {
 av_buffer_unref();
 }
 
+static void libdav1d_user_data_free(const uint8_t *data, void *opaque) {
+#if UINTPTR_MAX < UINT64_MAX
+av_free((void *)data);
+#endif
+}
+
 static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
 {
 Libdav1dContext *dav1d = c->priv_data;
@@ -191,6 +199,35 @@ static int libdav1d_receive_frame(AVCodecContext *c, 
AVFrame *frame)
 
 pkt.buf = NULL;
 av_packet_unref();
+
+#if UINTPTR_MAX >= UINT64_MAX
+if (c->reordered_opaque != (int64_t)(intptr_t)(const uint8_t 
*)NULL) {
+res = dav1d_data_wrap_user_data(data, (const uint8_t 
*)(intptr_t)c->reordered_opaque,
+libdav1d_user_data_free, NULL);
+if (res < 0) {
+dav1d_data_unref(data);
+return res;
+}
+}
+#else
+if (c->reordered_opaque != AV_NOPTS_VALUE) {
+int64_t *reordered_opaque = av_malloc(sizeof(int64_t));
+
+if (!reordered_opaque) {
+dav1d_data_unref(data);
+return AVERROR(ENOMEM);
+}
+
+*reordered_opaque = c->reordered_opaque;
+res = dav1d_data_wrap_user_data(data, (const uint8_t 
*)reordered_opaque,
+libdav1d_user_data_free, NULL);
+if (res < 0) {
+av_free(reordered_opaque);
+dav1d_data_unref(data);
+return res;
+}
+}
+#endif
 }
 }
 
@@ -260,7 +297,14 @@ static int libdav1d_receive_frame(AVCodecContext *c, 
AVFrame *frame)
 else
 frame->format = c->pix_fmt = pix_fmt[p->p.layout][p->seq_hdr->hbd];
 
-frame->reordered_opaque = c->reordered_opaque;
+#if UINTPTR_MAX >= UINT64_MAX
+frame->reordered_opaque = (int64_t)(intptr_t)p->m.user_data.data;
+#else
+if (p->m.user_data.data)
+memcpy(>reordered_opaque, p->m.user_data.data, 
sizeof(frame->reordered_opaque));
+else
+frame->reordered_opaque = AV_NOPTS_VALUE;
+#endif
 
 // match timestamps and packet size
 frame->pts = frame->best_effort_timestamp = p->m.timestamp;
-- 
2.20.1

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

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

Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque

[Andrey Semashev]

On 2019-10-18 10:01, Andrey Semashev wrote:

On 2019-10-18 02:16, James Almer wrote:

On 10/17/2019 7:46 PM, Andrey Semashev wrote:

On 2019-10-18 01:28, James Almer wrote:

On 10/17/2019 7:13 PM, Andrey Semashev wrote:

On 2019-10-17 23:11, James Almer wrote:

Actually reorder the values.

Should effectively fix ticket #8300.

Signed-off-by: James Almer 
---
    libavcodec/libdav1d.c | 21 -
    1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext
*c, AVFrame *frame)
      pkt.buf = NULL;
    av_packet_unref();
+
+    if (c->reordered_opaque != AV_NOPTS_VALUE) {


I'm not sure this comparison is valid. The default value of
reordered_opaque is 0, and there seems to be no convention whatsoever
about what this value represents (i.e. its semantics are completely
user-defined). I think, this check needs to be removed and the code
below should execute for any reordered_opaque values.


AVCodecContext->reordered_opaque is by default AV_NOPTS_VALUE, as 
set in

avcodec_alloc_context3(). This field is normally used for timestamps
(despite not being the only use), and 0 is a valid timestamp, so that
can't be considered a "not set" value.


Ok, I see. I was looking at AVFrame initialization, which sets it to 0
by default.


And I'd rather not make this code unconditional. It's an allocation per
frame in a zero copy optimized decoder, and i don't want that overhead
when reordered_opaque is rarely going to be used.

Fwiw, timestamps are properly reordered by libdav1d in this same
function and propagated in the output frame. reordered_opaque is not
really needed for them.


FWIW, I'm the reporter of #8300 and our main reason for using
reordered_opaque instead of pts is that we don't want to mess with
timestamp conversion between our time base and whatever time_base
libavcodec might select for a given codec. So, we use reordered_opaque
universally, and it just happened to break with libdav1d.

Testing for AV_NOPTS_VALUE is ok in our particular case, though I had
the impression that ffmpeg is not supposed to interpret
reordered_opaque, including not assume it contains a timestamp.


Unfortunately you're right, and the check is probably not proper use of
the field, even if valid for pretty much any normal use case for it.

Will remove it, and simply deal with the malloc overhead.


You could avoid malloc completely on 64-bit systems by passing 
reordered_opaque inside the pointer to user data. It's not pretty, but 
it would get the job done. 32-bit systems would still have to malloc, 
unfortunately.


I've posted v3 patch with what I had in mind.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] libavcodec/h261dec: Fix keyframe markup and frame skipping.

[Andrey Semashev]

On 2019-10-26 14:05, Andrey Semashev wrote:

The decoder never marks pictures as I-frames, which results in no
keyframe indication and incorrect frame skipping, in cases when
keyframes should be decoded.

This commit works around this decoder limitation and marks I-frames
and keyframes based on "freeze picture release" bit in h261 picture
header. This reflects h261enc behavior.


So, is this patch acceptable? If yes, could someone merge it please?


---
  libavcodec/h261.h|  1 +
  libavcodec/h261dec.c | 27 ++-
  2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/libavcodec/h261.h b/libavcodec/h261.h
index 399a404b2b..6662d38d6d 100644
--- a/libavcodec/h261.h
+++ b/libavcodec/h261.h
@@ -37,6 +37,7 @@
  typedef struct H261Context {
  MpegEncContext s;
  
+int freeze_picture_release; // 1 if freeze picture release bit is set in the picture header

  int current_mba;
  int mba_diff;
  int mtype;
diff --git a/libavcodec/h261dec.c b/libavcodec/h261dec.c
index 14a874c45d..3b1711a21d 100644
--- a/libavcodec/h261dec.c
+++ b/libavcodec/h261dec.c
@@ -502,9 +502,9 @@ static int h261_decode_picture_header(H261Context *h)
  s->avctx->framerate = (AVRational) { 3, 1001 };
  
  /* PTYPE starts here */

-skip_bits1(>gb); /* split screen off */
-skip_bits1(>gb); /* camera  off */
-skip_bits1(>gb); /* freeze picture release off */
+skip_bits1(>gb); /* split screen indicator */
+skip_bits1(>gb); /* document camera indicator */
+h->freeze_picture_release = get_bits1(>gb); /* freeze picture release */
  
  format = get_bits1(>gb);
  
@@ -532,7 +532,8 @@ static int h261_decode_picture_header(H261Context *h)
  
  /* H.261 has no I-frames, but if we pass AV_PICTURE_TYPE_I for the first

   * frame, the codec crashes if it does not contain all I-blocks
- * (e.g. when a packet is lost). */
+ * (e.g. when a packet is lost). We will fix the picture type in the
+ * output frame based on h->freeze_picture_release later. */
  s->pict_type = AV_PICTURE_TYPE_P;
  
  h->gob_number = 0;

@@ -590,6 +591,7 @@ static int h261_decode_frame(AVCodecContext *avctx, void 
*data,
  H261Context *h = avctx->priv_data;
  MpegEncContext *s  = >s;
  int ret;
+enum AVPictureType pict_type;
  AVFrame *pict = data;
  
  ff_dlog(avctx, "*frame %d size=%d\n", avctx->frame_number, buf_size);

@@ -630,15 +632,17 @@ retry:
  goto retry;
  }
  
-// for skipping the frame

-s->current_picture.f->pict_type = s->pict_type;
-s->current_picture.f->key_frame = s->pict_type == AV_PICTURE_TYPE_I;
+// for skipping the frame and keyframe markup
+pict_type = h->freeze_picture_release ? AV_PICTURE_TYPE_I : s->pict_type;
  
-if ((avctx->skip_frame >= AVDISCARD_NONREF && s->pict_type == AV_PICTURE_TYPE_B) ||

-(avctx->skip_frame >= AVDISCARD_NONKEY && s->pict_type != 
AV_PICTURE_TYPE_I) ||
+if ((avctx->skip_frame >= AVDISCARD_NONREF && pict_type == 
AV_PICTURE_TYPE_B) ||
+(avctx->skip_frame >= AVDISCARD_NONKEY && pict_type != 
AV_PICTURE_TYPE_I) ||
   avctx->skip_frame >= AVDISCARD_ALL)
  return get_consumed_bytes(s, buf_size);
  
+s->current_picture.f->pict_type = s->pict_type;

+s->current_picture.f->key_frame = s->pict_type == AV_PICTURE_TYPE_I;
+
  if (ff_mpv_frame_start(s, avctx) < 0)
  return -1;
  
@@ -660,6 +664,11 @@ retry:
  
  if ((ret = av_frame_ref(pict, s->current_picture_ptr->f)) < 0)

  return ret;
+
+// fix picture type and correctly mark keyframes
+pict->pict_type = pict_type;
+pict->key_frame = pict_type == AV_PICTURE_TYPE_I;
+
  ff_print_debug_info(s, s->current_picture_ptr, pict);
  
  *got_frame = 1;




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

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

Re: [FFmpeg-devel] [PATCH v2] avcodec/libdav1d: fix setting AVFrame reordered_opaque

[Andrey Semashev]

On 2019-10-18 02:18, James Almer wrote:

Actually reorder the values.

Should effectively fix ticket #8300.

Signed-off-by: James Almer 
---
Now unconditionally propagating the field, since checking its value is
not correct usage of the field.


James, do you still plan working on this patch?


  libavcodec/libdav1d.c | 25 -
  1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..1793c9e4f0 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -164,6 +164,11 @@ static void libdav1d_data_free(const uint8_t *data, void 
*opaque) {
  av_buffer_unref();
  }
  
+static void libdav1d_user_data_free(const uint8_t *data, void *opaque) {

+av_assert2(data == opaque);
+av_free(opaque);
+}
+
  static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
  {
  Libdav1dContext *dav1d = c->priv_data;
@@ -179,6 +184,8 @@ static int libdav1d_receive_frame(AVCodecContext *c, 
AVFrame *frame)
  return res;
  
  if (pkt.size) {

+uint8_t *reordered_opaque;
+
  res = dav1d_data_wrap(data, pkt.data, pkt.size, 
libdav1d_data_free, pkt.buf);
  if (res < 0) {
  av_packet_unref();
@@ -191,6 +198,21 @@ static int libdav1d_receive_frame(AVCodecContext *c, 
AVFrame *frame)
  
  pkt.buf = NULL;

  av_packet_unref();
+
+reordered_opaque = av_malloc(sizeof(c->reordered_opaque));
+if (!reordered_opaque) {
+dav1d_data_unref(data);
+return AVERROR(ENOMEM);
+}
+
+memcpy(reordered_opaque, >reordered_opaque, 
sizeof(c->reordered_opaque));
+res = dav1d_data_wrap_user_data(data, reordered_opaque,
+libdav1d_user_data_free, 
reordered_opaque);
+if (res < 0) {
+av_free(reordered_opaque);
+dav1d_data_unref(data);
+return res;
+}
  }
  }
  
@@ -260,7 +282,8 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)

  else
  frame->format = c->pix_fmt = pix_fmt[p->p.layout][p->seq_hdr->hbd];
  
-frame->reordered_opaque = c->reordered_opaque;

+av_assert0(p->m.user_data.data);
+memcpy(>reordered_opaque, p->m.user_data.data, 
sizeof(frame->reordered_opaque));
  
  // match timestamps and packet size

  frame->pts = frame->best_effort_timestamp = p->m.timestamp;



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

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

Re: [FFmpeg-devel] [PATCH v2] avcodec/libdav1d: fix setting AVFrame reordered_opaque

[Andrey Semashev]

On 2019-10-22 17:09, James Almer wrote:

On 10/22/2019 11:01 AM, Andrey Semashev wrote:

On 2019-10-18 02:18, James Almer wrote:

Actually reorder the values.

Should effectively fix ticket #8300.

Signed-off-by: James Almer 
---
Now unconditionally propagating the field, since checking its value is
not correct usage of the field.


James, do you still plan working on this patch?


Yes, but i have no way to test it. Can you confirm the current
implementation in the tree misbehaves, and that this approach corrects it?


No, I don't have a test video file that would cause frame reordering. At 
least I don't think any of my files cause it.


I tried my v3 patch with the files I have, it worked as intended.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v3] avcodec/libdav1d: fix setting AVFrame reordered_opaque

[Andrey Semashev]

On 2019-10-18 15:52, James Almer wrote:

On 10/18/2019 7:22 AM, Andrey Semashev wrote:

Actually reorder the values.

Should effectively fix ticket #8300.

Signed-off-by: James Almer 

Updated to avoid extra memory allocations on 64-bit platforms.

Signed-off-by: Andrey Semashev 


Please stop sending patches by other people with the authorship changed,
even if you modified it somewhat.

Suggest changes, or ask if I'm ok with you taking over the patch.
Anything else will more likely than not be seen as rude by anyone.


Sorry, I didn't mean to offend anyone. I honestly don't understand the 
problem. I didn't claim the credit for the original change and I did 
preserve your comment and Signed-off-by, didn't I? Is there something 
else that I should have done?


My aim was to demonstrate my suggestion and perhaps simplify your work. 
I don't care if my changes are taken or modified, credited or not. If 
that is not wanted, that is fine, I won't send any more patches.

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

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

Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque

[Andrey Semashev]

On 2019-10-17 23:11, James Almer wrote:

Actually reorder the values.

Should effectively fix ticket #8300.

Signed-off-by: James Almer 
---
  libavcodec/libdav1d.c | 21 -
  1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext *c, 
AVFrame *frame)
  
  pkt.buf = NULL;

  av_packet_unref();
+
+if (c->reordered_opaque != AV_NOPTS_VALUE) {
+AVBufferRef *reordered_opaque = 
av_buffer_alloc(sizeof(c->reordered_opaque));
+
+if (!reordered_opaque) {
+dav1d_data_unref(data);
+return AVERROR(ENOMEM);
+}
+
+*reordered_opaque->data = c->reordered_opaque;


This slices int64_t to uint8_t. Should memcpy instead.


+res = dav1d_data_wrap_user_data(data, reordered_opaque->data,
+libdav1d_data_free, 
reordered_opaque);
+if (res < 0) {
+av_buffer_unref(_opaque);
+dav1d_data_unref(data);
+return res;
+}
+}
  }
  }
  
@@ -260,7 +278,8 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)

  else
  frame->format = c->pix_fmt = pix_fmt[p->p.layout][p->seq_hdr->hbd];
  
-frame->reordered_opaque = c->reordered_opaque;

+if (p->m.user_data.data)
+frame->reordered_opaque = *(int64_t *)p->m.user_data.data;
  
  // match timestamps and packet size

  frame->pts = frame->best_effort_timestamp = p->m.timestamp;



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

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

Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque

[Andrey Semashev]

On 2019-10-18 00:43, Andrey Semashev wrote:

On 2019-10-17 23:11, James Almer wrote:

Actually reorder the values.

Should effectively fix ticket #8300.

Signed-off-by: James Almer 
---
  libavcodec/libdav1d.c | 21 -
  1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext 
*c, AVFrame *frame)

  pkt.buf = NULL;
  av_packet_unref();
+
+    if (c->reordered_opaque != AV_NOPTS_VALUE) {
+    AVBufferRef *reordered_opaque = 
av_buffer_alloc(sizeof(c->reordered_opaque));

+
+    if (!reordered_opaque) {
+    dav1d_data_unref(data);
+    return AVERROR(ENOMEM);
+    }
+
+    *reordered_opaque->data = c->reordered_opaque;


This slices int64_t to uint8_t. Should memcpy instead.


Also, would it be possible to save at least one allocation by saving a 
pointer to int64_t or a struct containing int64_t? AVBufferRef seems 
unnecessary.


+    res = dav1d_data_wrap_user_data(data, 
reordered_opaque->data,
+    libdav1d_data_free, 
reordered_opaque);

+    if (res < 0) {
+    av_buffer_unref(_opaque);
+    dav1d_data_unref(data);
+    return res;
+    }
+    }
  }
  }
@@ -260,7 +278,8 @@ static int libdav1d_receive_frame(AVCodecContext 
*c, AVFrame *frame)

  else
  frame->format = c->pix_fmt = 
pix_fmt[p->p.layout][p->seq_hdr->hbd];

-    frame->reordered_opaque = c->reordered_opaque;
+    if (p->m.user_data.data)
+    frame->reordered_opaque = *(int64_t *)p->m.user_data.data;
  // match timestamps and packet size
  frame->pts = frame->best_effort_timestamp = p->m.timestamp;





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

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

Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque

[Andrey Semashev]

On 2019-10-17 23:11, James Almer wrote:

Actually reorder the values.

Should effectively fix ticket #8300.

Signed-off-by: James Almer 
---
  libavcodec/libdav1d.c | 21 -
  1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext *c, 
AVFrame *frame)
  
  pkt.buf = NULL;

  av_packet_unref();
+
+if (c->reordered_opaque != AV_NOPTS_VALUE) {


I'm not sure this comparison is valid. The default value of 
reordered_opaque is 0, and there seems to be no convention whatsoever 
about what this value represents (i.e. its semantics are completely 
user-defined). I think, this check needs to be removed and the code 
below should execute for any reordered_opaque values.



+AVBufferRef *reordered_opaque = 
av_buffer_alloc(sizeof(c->reordered_opaque));
+
+if (!reordered_opaque) {
+dav1d_data_unref(data);
+return AVERROR(ENOMEM);
+}
+
+*reordered_opaque->data = c->reordered_opaque;
+res = dav1d_data_wrap_user_data(data, reordered_opaque->data,
+libdav1d_data_free, 
reordered_opaque);
+if (res < 0) {
+av_buffer_unref(_opaque);
+dav1d_data_unref(data);
+return res;
+}
+}
  }
  }
  
@@ -260,7 +278,8 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)

  else
  frame->format = c->pix_fmt = pix_fmt[p->p.layout][p->seq_hdr->hbd];
  
-frame->reordered_opaque = c->reordered_opaque;

+if (p->m.user_data.data)
+frame->reordered_opaque = *(int64_t *)p->m.user_data.data;
  
  // match timestamps and packet size

  frame->pts = frame->best_effort_timestamp = p->m.timestamp;



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

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

Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque

[Andrey Semashev]

On 2019-10-18 01:27, Hendrik Leppkes wrote:

On Fri, Oct 18, 2019 at 12:13 AM Andrey Semashev
 wrote:


On 2019-10-17 23:11, James Almer wrote:

Actually reorder the values.

Should effectively fix ticket #8300.

Signed-off-by: James Almer 
---
   libavcodec/libdav1d.c | 21 -
   1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext *c, 
AVFrame *frame)

   pkt.buf = NULL;
   av_packet_unref();
+
+if (c->reordered_opaque != AV_NOPTS_VALUE) {


I'm not sure this comparison is valid. The default value of
reordered_opaque is 0, and there seems to be no convention whatsoever
about what this value represents (i.e. its semantics are completely
user-defined). I think, this check needs to be removed and the code
below should execute for any reordered_opaque values.



AV_NOPTS_VALUE is the default value of that field in AVCodecContext
(see init_context_defaults in avcodec\options.c), so we can easily
avoid an allocation while conveying the same value, which will happen
a lot since the field isn't used everywhere.
I'm not sure if AVFrame also defaults to that value, but even if it
doesn't, further down it could just set the field to that if no value
is provided.


AVFrame::reordered_opaque is zero by default, I looked at 
get_frame_defaults.

___
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] avcodec/libdav1d: fix setting AVFrame reordered_opaque

[Andrey Semashev]

Actually reorder the values.

Should effectively fix ticket #8300.
---
 libavcodec/libdav1d.c | 26 +-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..774e741db8 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -25,6 +25,7 @@
 #include "libavutil/mastering_display_metadata.h"
 #include "libavutil/imgutils.h"
 #include "libavutil/opt.h"
+#include "libavutil/mem.h"
 
 #include "avcodec.h"
 #include "decode.h"
@@ -164,6 +165,10 @@ static void libdav1d_data_free(const uint8_t *data, void 
*opaque) {
 av_buffer_unref();
 }
 
+static void libdav1d_user_data_free(const uint8_t *data, void *opaque) {
+av_free(data);
+}
+
 static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
 {
 Libdav1dContext *dav1d = c->priv_data;
@@ -179,6 +184,8 @@ static int libdav1d_receive_frame(AVCodecContext *c, 
AVFrame *frame)
 return res;
 
 if (pkt.size) {
+int64_t *reordered_opaque;
+
 res = dav1d_data_wrap(data, pkt.data, pkt.size, 
libdav1d_data_free, pkt.buf);
 if (res < 0) {
 av_packet_unref();
@@ -191,6 +198,22 @@ static int libdav1d_receive_frame(AVCodecContext *c, 
AVFrame *frame)
 
 pkt.buf = NULL;
 av_packet_unref();
+
+reordered_opaque = av_malloc(sizeof(int64_t));
+
+if (!reordered_opaque) {
+dav1d_data_unref(data);
+return AVERROR(ENOMEM);
+}
+
+*reordered_opaque = c->reordered_opaque;
+res = dav1d_data_wrap_user_data(data, (const uint8_t 
*)reordered_opaque,
+libdav1d_user_data_free, NULL);
+if (res < 0) {
+av_free(reordered_opaque);
+dav1d_data_unref(data);
+return res;
+}
 }
 }
 
@@ -260,7 +283,8 @@ static int libdav1d_receive_frame(AVCodecContext *c, 
AVFrame *frame)
 else
 frame->format = c->pix_fmt = pix_fmt[p->p.layout][p->seq_hdr->hbd];
 
-frame->reordered_opaque = c->reordered_opaque;
+if (p->m.user_data.data)
+frame->reordered_opaque = *(int64_t *)p->m.user_data.data;
 
 // match timestamps and packet size
 frame->pts = frame->best_effort_timestamp = p->m.timestamp;
-- 
2.20.1

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

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

Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque

[Andrey Semashev]

Please, ignore this patch.

On 2019-10-18 01:44, James Almer wrote:

On 10/17/2019 7:34 PM, Andrey Semashev wrote:

Actually reorder the values.

Should effectively fix ticket #8300.


Not sure why you decided to send a modified patch by another dev, and
with the author name changed, but that's not ok.


---
  libavcodec/libdav1d.c | 26 +-
  1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..774e741db8 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -25,6 +25,7 @@
  #include "libavutil/mastering_display_metadata.h"
  #include "libavutil/imgutils.h"
  #include "libavutil/opt.h"
+#include "libavutil/mem.h"
  
  #include "avcodec.h"

  #include "decode.h"
@@ -164,6 +165,10 @@ static void libdav1d_data_free(const uint8_t *data, void 
*opaque) {
  av_buffer_unref();
  }
  
+static void libdav1d_user_data_free(const uint8_t *data, void *opaque) {

+av_free(data);


This will generate a warning about dropped const qualifier.


+}
+
  static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
  {
  Libdav1dContext *dav1d = c->priv_data;
@@ -179,6 +184,8 @@ static int libdav1d_receive_frame(AVCodecContext *c, 
AVFrame *frame)
  return res;
  
  if (pkt.size) {

+int64_t *reordered_opaque;
+
  res = dav1d_data_wrap(data, pkt.data, pkt.size, 
libdav1d_data_free, pkt.buf);
  if (res < 0) {
  av_packet_unref();
@@ -191,6 +198,22 @@ static int libdav1d_receive_frame(AVCodecContext *c, 
AVFrame *frame)
  
  pkt.buf = NULL;

  av_packet_unref();
+
+reordered_opaque = av_malloc(sizeof(int64_t));
+
+if (!reordered_opaque) {
+dav1d_data_unref(data);
+return AVERROR(ENOMEM);
+}
+
+*reordered_opaque = c->reordered_opaque;
+res = dav1d_data_wrap_user_data(data, (const uint8_t 
*)reordered_opaque,
+libdav1d_user_data_free, NULL);
+if (res < 0) {
+av_free(reordered_opaque);
+dav1d_data_unref(data);
+return res;
+}


As i said, i don't want this done unconditionally. It's not going to be
used in 99% of cases.


  }
  }
  
@@ -260,7 +283,8 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)

  else
  frame->format = c->pix_fmt = pix_fmt[p->p.layout][p->seq_hdr->hbd];
  
-frame->reordered_opaque = c->reordered_opaque;

+if (p->m.user_data.data)
+frame->reordered_opaque = *(int64_t *)p->m.user_data.data;
  
  // match timestamps and packet size

  frame->pts = frame->best_effort_timestamp = p->m.timestamp;



___
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".

Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque

[Andrey Semashev]

On 2019-10-18 01:28, James Almer wrote:

On 10/17/2019 7:13 PM, Andrey Semashev wrote:

On 2019-10-17 23:11, James Almer wrote:

Actually reorder the values.

Should effectively fix ticket #8300.

Signed-off-by: James Almer 
---
   libavcodec/libdav1d.c | 21 -
   1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext
*c, AVFrame *frame)
     pkt.buf = NULL;
   av_packet_unref();
+
+    if (c->reordered_opaque != AV_NOPTS_VALUE) {


I'm not sure this comparison is valid. The default value of
reordered_opaque is 0, and there seems to be no convention whatsoever
about what this value represents (i.e. its semantics are completely
user-defined). I think, this check needs to be removed and the code
below should execute for any reordered_opaque values.


AVCodecContext->reordered_opaque is by default AV_NOPTS_VALUE, as set in
avcodec_alloc_context3(). This field is normally used for timestamps
(despite not being the only use), and 0 is a valid timestamp, so that
can't be considered a "not set" value.


Ok, I see. I was looking at AVFrame initialization, which sets it to 0 
by default.



And I'd rather not make this code unconditional. It's an allocation per
frame in a zero copy optimized decoder, and i don't want that overhead
when reordered_opaque is rarely going to be used.

Fwiw, timestamps are properly reordered by libdav1d in this same
function and propagated in the output frame. reordered_opaque is not
really needed for them.


FWIW, I'm the reporter of #8300 and our main reason for using 
reordered_opaque instead of pts is that we don't want to mess with 
timestamp conversion between our time base and whatever time_base 
libavcodec might select for a given codec. So, we use reordered_opaque 
universally, and it just happened to break with libdav1d.


Testing for AV_NOPTS_VALUE is ok in our particular case, though I had 
the impression that ffmpeg is not supposed to interpret 
reordered_opaque, including not assume it contains a timestamp.


In any case, I've sent my version of the patch without the check before 
reading the replies. Feel free to ignore it, if you want to keep the check.

___
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] libavcodec/h261dec: Fix keyframe markup and frame skipping.

[Andrey Semashev]

The decoder never marks pictures as I-frames, which results in no
keyframe indication and incorrect frame skipping, in cases when
keyframes should be decoded.

This commit works around this decoder limitation and marks I-frames
and keyframes based on "freeze picture release" bit in h261 picture
header. This reflects h261enc behavior.
---
 libavcodec/h261.h|  1 +
 libavcodec/h261dec.c | 27 ++-
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/libavcodec/h261.h b/libavcodec/h261.h
index 399a404b2b..6662d38d6d 100644
--- a/libavcodec/h261.h
+++ b/libavcodec/h261.h
@@ -37,6 +37,7 @@
 typedef struct H261Context {
 MpegEncContext s;
 
+int freeze_picture_release; // 1 if freeze picture release bit is set in 
the picture header
 int current_mba;
 int mba_diff;
 int mtype;
diff --git a/libavcodec/h261dec.c b/libavcodec/h261dec.c
index 14a874c45d..3b1711a21d 100644
--- a/libavcodec/h261dec.c
+++ b/libavcodec/h261dec.c
@@ -502,9 +502,9 @@ static int h261_decode_picture_header(H261Context *h)
 s->avctx->framerate = (AVRational) { 3, 1001 };
 
 /* PTYPE starts here */
-skip_bits1(>gb); /* split screen off */
-skip_bits1(>gb); /* camera  off */
-skip_bits1(>gb); /* freeze picture release off */
+skip_bits1(>gb); /* split screen indicator */
+skip_bits1(>gb); /* document camera indicator */
+h->freeze_picture_release = get_bits1(>gb); /* freeze picture release */
 
 format = get_bits1(>gb);
 
@@ -532,7 +532,8 @@ static int h261_decode_picture_header(H261Context *h)
 
 /* H.261 has no I-frames, but if we pass AV_PICTURE_TYPE_I for the first
  * frame, the codec crashes if it does not contain all I-blocks
- * (e.g. when a packet is lost). */
+ * (e.g. when a packet is lost). We will fix the picture type in the
+ * output frame based on h->freeze_picture_release later. */
 s->pict_type = AV_PICTURE_TYPE_P;
 
 h->gob_number = 0;
@@ -590,6 +591,7 @@ static int h261_decode_frame(AVCodecContext *avctx, void 
*data,
 H261Context *h = avctx->priv_data;
 MpegEncContext *s  = >s;
 int ret;
+enum AVPictureType pict_type;
 AVFrame *pict = data;
 
 ff_dlog(avctx, "*frame %d size=%d\n", avctx->frame_number, buf_size);
@@ -630,15 +632,17 @@ retry:
 goto retry;
 }
 
-// for skipping the frame
-s->current_picture.f->pict_type = s->pict_type;
-s->current_picture.f->key_frame = s->pict_type == AV_PICTURE_TYPE_I;
+// for skipping the frame and keyframe markup
+pict_type = h->freeze_picture_release ? AV_PICTURE_TYPE_I : s->pict_type;
 
-if ((avctx->skip_frame >= AVDISCARD_NONREF && s->pict_type == 
AV_PICTURE_TYPE_B) ||
-(avctx->skip_frame >= AVDISCARD_NONKEY && s->pict_type != 
AV_PICTURE_TYPE_I) ||
+if ((avctx->skip_frame >= AVDISCARD_NONREF && pict_type == 
AV_PICTURE_TYPE_B) ||
+(avctx->skip_frame >= AVDISCARD_NONKEY && pict_type != 
AV_PICTURE_TYPE_I) ||
  avctx->skip_frame >= AVDISCARD_ALL)
 return get_consumed_bytes(s, buf_size);
 
+s->current_picture.f->pict_type = s->pict_type;
+s->current_picture.f->key_frame = s->pict_type == AV_PICTURE_TYPE_I;
+
 if (ff_mpv_frame_start(s, avctx) < 0)
 return -1;
 
@@ -660,6 +664,11 @@ retry:
 
 if ((ret = av_frame_ref(pict, s->current_picture_ptr->f)) < 0)
 return ret;
+
+// fix picture type and correctly mark keyframes
+pict->pict_type = pict_type;
+pict->key_frame = pict_type == AV_PICTURE_TYPE_I;
+
 ff_print_debug_info(s, s->current_picture_ptr, pict);
 
 *got_frame = 1;
-- 
2.20.1

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

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

Re: [FFmpeg-devel] [PATCH] libavcodec/h261dec: Fix keyframe markup and frame skipping.

[Andrey Semashev]

On 2019-10-26 15:49, Carl Eugen Hoyos wrote:

Am Sa., 26. Okt. 2019 um 13:12 Uhr schrieb Andrey Semashev
:


The decoder never marks pictures as I-frames, which results in no
keyframe indication and incorrect frame skipping, in cases when
keyframes should be decoded.

This commit works around this decoder limitation and marks I-frames
and keyframes based on "freeze picture release" bit in h261 picture
header. This reflects h261enc behavior.



diff --git a/libavcodec/h261dec.c b/libavcodec/h261dec.c
index 14a874c45d..3b1711a21d 100644
--- a/libavcodec/h261dec.c
+++ b/libavcodec/h261dec.c
@@ -502,9 +502,9 @@ static int h261_decode_picture_header(H261Context *h)
  s->avctx->framerate = (AVRational) { 3, 1001 };

  /* PTYPE starts here */
-skip_bits1(>gb); /* split screen off */
-skip_bits1(>gb); /* camera  off */
-skip_bits1(>gb); /* freeze picture release off */
+skip_bits1(>gb); /* split screen indicator */
+skip_bits1(>gb); /* document camera indicator */
+h->freeze_picture_release = get_bits1(>gb); /* freeze picture release */

  format = get_bits1(>gb);

@@ -532,7 +532,8 @@ static int h261_decode_picture_header(H261Context *h)

  /* H.261 has no I-frames, but if we pass AV_PICTURE_TYPE_I for the first
   * frame, the codec crashes if it does not contain all I-blocks
- * (e.g. when a packet is lost). */
+ * (e.g. when a packet is lost). We will fix the picture type in the
+ * output frame based on h->freeze_picture_release later. */
  s->pict_type = AV_PICTURE_TYPE_P;


Why can't you use freeze_picture_release here?


The comment says the decoder may crash on some input. I don't know if 
this is true and I don't have any test files not produced by ffmpeg 
itself to test.

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

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

Re: [FFmpeg-devel] [PATCH] libavcodec/h261dec: Fix keyframe markup and frame skipping.

[Andrey Semashev]

On 2019-10-26 21:15, Michael Niedermayer wrote:

On Sat, Oct 26, 2019 at 02:05:27PM +0300, Andrey Semashev wrote:

The decoder never marks pictures as I-frames, which results in no
keyframe indication and incorrect frame skipping, in cases when
keyframes should be decoded.

This commit works around this decoder limitation and marks I-frames
and keyframes based on "freeze picture release" bit in h261 picture
header. This reflects h261enc behavior.
---
  libavcodec/h261.h|  1 +
  libavcodec/h261dec.c | 27 ++-
  2 files changed, 19 insertions(+), 9 deletions(-)


If the goal is correctly recognizing I frames then checking if all
blocks are intra should be the most reliable


In my case, the goal is to know when a keyframe is received, i.e. when 
the receiver can be reasonably sure it can start displaying/processing 
received frames. Including after some frames lost in transmission.


According to H.261 spec, "freeze picture release" bit is what is 
intended to mark keyframes. To quote section 4.3.3:



A signal from an encoder which has responded to a fast update request 
and allows a decoder to exit from its freeze picture mode and display 
decoded pictures in the normal manner.



I'll reiterate that h261enc marks keyframes with this bit.


that wont work for skiping though as it requires decoding


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

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

Re: [FFmpeg-devel] [PATCH v2] avcodec/libdav1d: fix setting AVFrame reordered_opaque

[Andrey Semashev]

On 2019-10-22 17:14, Andrey Semashev wrote:

On 2019-10-22 17:09, James Almer wrote:

On 10/22/2019 11:01 AM, Andrey Semashev wrote:

On 2019-10-18 02:18, James Almer wrote:

Actually reorder the values.

Should effectively fix ticket #8300.

Signed-off-by: James Almer 
---
Now unconditionally propagating the field, since checking its value is
not correct usage of the field.


James, do you still plan working on this patch?


Yes, but i have no way to test it. Can you confirm the current
implementation in the tree misbehaves, and that this approach corrects 
it?


No, I don't have a test video file that would cause frame reordering. At 
least I don't think any of my files cause it.


I tried my v3 patch with the files I have, it worked as intended.


Actually, no, one of the files causes a delay for one frame, so I can 
test it. Your v2 and my v3 patches work (i.e. the decoded 
reordered_opaque lags behind input pts for one frame).

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

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

Re: [FFmpeg-devel] avcodec/aacenc: Compatibility with Bluetooth A2DP headsets

[Andrey Semashev]

On 2019-12-24 01:57, Andrey Semashev wrote:

Hi,

I'm trying to add support for AAC to PulseAudio with regard to Bluetooth 
A2DP. I'm using libavcodec to encode AAC content and then packetize it 
into RTP MP4-LATM packets, as required by A2DP spec. My current 
implementation allows to switch between libfdk-aac and built-in AAC 
encoder backends.


With libfdk-aac, everything's working well, the encoded audio is 
correctly played by the BT headset (EOZ Air). But with the built-in 
encoder I can hear occasional audio drop outs and artefacts like clicks 
and weird stereo effects. I'm assuming this is because some decoding 
errors on the headset, but obviously I don't have any kind of debug info 
from it. When the same content is played (e.g. the same song) on 
different runs, the artefacts appear at roughly the same places, so I'm 
assuming that the problem is input content-related.


Given that libfdk-aac output is played without artefacts by the device, 
I'm assuming my packetization code is correct. I also tried to debug and 
analyze its output and didn't find anything out of order. This makes me 
believe the problem is in the built-in encoder.


How should I proceed about this problem? What kind of information would 
be useful to the aacenc developer(s) to debug this? Is there any 
information as to how conforming aacenc is to the AAC spec and other, 
especially hardware implementations?


For a start, here is my code:

https://gitlab.freedesktop.org/andrey.semashev/pulseaudio/blob/a2dp-codecs-aac/src/modules/bluetooth/a2dp-codec-aac.c 



libavcodec encoder context initialization is at:

https://gitlab.freedesktop.org/andrey.semashev/pulseaudio/blob/a2dp-codecs-aac/src/modules/bluetooth/a2dp-codec-aac.c#L685 



Thanks.


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

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

Re: [FFmpeg-devel] avcodec/aacenc: Compatibility with Bluetooth A2DP headsets

[Andrey Semashev]

On 2020-01-03 09:14, Kieran Kunhya wrote:



Thanks.


Ping?



Have you tried using our LATM mux?


No, I'm trying to avoid libavformat dependency. I'm sure my LATM code is 
fine because it works with libfdk-aac.

___
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] avcodec/aacenc: Compatibility with Bluetooth A2DP headsets

[Andrey Semashev]

Hi,

I'm trying to add support for AAC to PulseAudio with regard to Bluetooth 
A2DP. I'm using libavcodec to encode AAC content and then packetize it 
into RTP MP4-LATM packets, as required by A2DP spec. My current 
implementation allows to switch between libfdk-aac and built-in AAC 
encoder backends.


With libfdk-aac, everything's working well, the encoded audio is 
correctly played by the BT headset (EOZ Air). But with the built-in 
encoder I can hear occasional audio drop outs and artefacts like clicks 
and weird stereo effects. I'm assuming this is because some decoding 
errors on the headset, but obviously I don't have any kind of debug info 
from it. When the same content is played (e.g. the same song) on 
different runs, the artefacts appear at roughly the same places, so I'm 
assuming that the problem is input content-related.


Given that libfdk-aac output is played without artefacts by the device, 
I'm assuming my packetization code is correct. I also tried to debug and 
analyze its output and didn't find anything out of order. This makes me 
believe the problem is in the built-in encoder.


How should I proceed about this problem? What kind of information would 
be useful to the aacenc developer(s) to debug this? Is there any 
information as to how conforming aacenc is to the AAC spec and other, 
especially hardware implementations?


For a start, here is my code:

https://gitlab.freedesktop.org/andrey.semashev/pulseaudio/blob/a2dp-codecs-aac/src/modules/bluetooth/a2dp-codec-aac.c

libavcodec encoder context initialization is at:

https://gitlab.freedesktop.org/andrey.semashev/pulseaudio/blob/a2dp-codecs-aac/src/modules/bluetooth/a2dp-codec-aac.c#L685

Thanks.
___
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] libavformat/hlsenc: Remove duplicate close of the output stream.

[Andrey Semashev]

The result of the first close attempt is ignored and may be lost. By removing
it we ensure the close result code is properly analyzed.
---
 libavformat/hlsenc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 71fa3db060..88b58a1ba8 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -2631,7 +2631,6 @@ static int hls_write_trailer(struct AVFormatContext *s)
 goto failed;
 
 vs->size = range_length;
-hlsenc_io_close(s, >out, filename);
 ret = hlsenc_io_close(s, >out, filename);
 if (ret < 0) {
 av_log(s, AV_LOG_WARNING, "upload segment failed, will retry with 
a new http session.\n");
-- 
2.25.1

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

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

Re: [FFmpeg-devel] [PATCH] libavformat/hlsenc: Remove duplicate close of the output stream.

[Andrey Semashev]

Ping?

On 2020-07-01 17:59, Andrey Semashev wrote:

The result of the first close attempt is ignored and may be lost. By removing
it we ensure the close result code is properly analyzed.
---
  libavformat/hlsenc.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 71fa3db060..88b58a1ba8 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -2631,7 +2631,6 @@ static int hls_write_trailer(struct AVFormatContext *s)
  goto failed;
  
  vs->size = range_length;

-hlsenc_io_close(s, >out, filename);
  ret = hlsenc_io_close(s, >out, filename);
  if (ret < 0) {
  av_log(s, AV_LOG_WARNING, "upload segment failed, will retry with a 
new http session.\n");



___
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".