[FFmpeg-devel] [PATCH] lavf/segment: provide a virtual AVIOContext representing all the segments

[Rodger Combs]

This allows the use of muxers like matroska, which attempt to seek even
when an AVIOContext doesn't set `seekable`, without concern for a rouge
seek leading the muxer to overwrite the wrong data in a later segment.
---
 doc/muxers.texi   |  17 
 libavformat/segment.c | 276 +-
 libavformat/version.h |   2 +-
 3 files changed, 223 insertions(+), 72 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 9ec2e31..2cfa1ec 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -1390,6 +1390,23 @@ argument must be a time duration specification, and 
defaults to 0.
 If enabled, write an empty segment if there are no packets during the period a
 segment would usually span. Otherwise, the segment will be filled with the next
 packet written. Defaults to @code{0}.
+
+@item individual_header_trailer @var{1|0}
+Write each segment as an individual distinct file in the underlying format.
+When this is set to @code{0}, the segments are treated as a single continuous
+stream. When set to @code{1} (the default), each individual segment receives
+a header and trailer, so they can be played independently.
+
+@item segment_header_filename @var{name}
+Write the global header to a separate file. Requires
+@var{individual_header_trailer} to be @code{0}. If not set, the global header
+will be written to the first file along with actual segment data.
+
+@item segment_seekback @var{1|0}
+Allow the muxer to seek back and overwrite data from previous segments. 
Requires
+@var{individual_header_trailer} to be @code{0}. If set to @code{0} (the 
default),
+the underlying muxer will be unable to seek back into previous segments, so 
they
+can be relied upon not to change once written.
 @end table
 
 @subsection Examples
diff --git a/libavformat/segment.c b/libavformat/segment.c
index 33a5cf0..4858e9c 100644
--- a/libavformat/segment.c
+++ b/libavformat/segment.c
@@ -51,8 +51,10 @@ typedef struct SegmentListEntry {
 int64_t start_pts;
 int64_t offset_pts;
 char *filename;
+char *full_filename;
 struct SegmentListEntry *next;
 int64_t last_duration;
+size_t start_offset;
 } SegmentListEntry;
 
 typedef enum {
@@ -125,7 +127,13 @@ typedef struct SegmentContext {
 
 SegmentListEntry cur_entry;
 SegmentListEntry *segment_list_entries;
+SegmentListEntry *segment_list_entries_all;
 SegmentListEntry *segment_list_entries_end;
+SegmentListEntry *segment_list_entry_writing;
+int seekback;  ///< allow seeking back to previous segments
+AVIOContext *cur_pb;   ///< current segment put-byte context
+size_t write_offset;
+size_t max_offset;
 } SegmentContext;
 
 static void print_csv_escaped_str(AVIOContext *ctx, const char *str)
@@ -144,6 +152,123 @@ static void print_csv_escaped_str(AVIOContext *ctx, const 
char *str)
 avio_w8(ctx, '"');
 }
 
+static int64_t virtual_seek(void *priv, int64_t target, int whence)
+{
+AVFormatContext *s = priv;
+SegmentContext *seg = s->priv_data;
+SegmentListEntry *it, *current = NULL;
+int64_t offset = target;
+int64_t ret;
+
+if (whence != SEEK_SET)
+return AVERROR(EINVAL);
+if (offset < 0)
+return AVERROR(EINVAL);
+
+if (offset >= seg->max_offset) {
+ff_format_io_close(s, >cur_pb);
+seg->write_offset = offset;
+return offset;
+}
+
+if (seg->cur_entry.start_offset <= offset) {
+current = >cur_entry;
+} else {
+for (it = seg->segment_list_entries_all; it; it = it->next) {
+if (it->start_offset <= offset)
+current = it;
+else if (it->start_offset > offset)
+break;
+}
+}
+
+offset -= current->start_offset;
+
+if (current != seg->segment_list_entry_writing) {
+int is_seekback = (current != >cur_entry) && 
seg->segment_list_entries;
+char *new_filename;
+AVIOContext *new_ctx = NULL;
+AVDictionary *options = NULL;
+
+if (!seg->seekback && is_seekback)
+return AVERROR(EINVAL);
+
+new_filename = current->full_filename;
+
+if (new_filename) {
+if (is_seekback)
+av_dict_set_int(, "truncate", 0, 0);
+if ((ret = s->io_open(s, _ctx, new_filename, AVIO_FLAG_WRITE, 
)) < 0) {
+av_log(s, AV_LOG_ERROR, "Failed to seek into segment '%s'\n", 
new_filename);
+return ret;
+}
+}
+
+ff_format_io_close(s, >cur_pb);
+seg->cur_pb = new_ctx;
+seg->segment_list_entry_writing = current;
+}
+
+if (seg->cur_pb)
+if ((ret = avio_seek(seg->cur_pb, offset, SEEK_SET)) < 0)
+return ret;
+
+seg->write_offset = offset;
+
+return target;
+}
+
+static int virtual_write(void *priv, uint8_t *buf, int buf_size)
+{
+AVFormatContext *s = priv;
+SegmentContext *seg = s->priv_data;
+int ret = 0;
+int written = 0;
+

[FFmpeg-devel] [PATCH] avcodec/qsvenc_h264: fix segfault when a53 SEI is not available

[Nablet Developer]

Signed-off-by: Nablet Developer 
---
 libavcodec/qsvenc_h264.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/qsvenc_h264.c b/libavcodec/qsvenc_h264.c
index c1f6003..f5b01bb 100644
--- a/libavcodec/qsvenc_h264.c
+++ b/libavcodec/qsvenc_h264.c
@@ -53,7 +53,7 @@ static int qsv_h264_set_encode_ctrl(AVCodecContext *avctx,
 int res;
 
 res = ff_alloc_a53_sei(frame, sizeof(mfxPayload) + 2, 
(void**), _size);
-if (res < 0)
+if (res < 0 || !payload)
 return res;
 
 sei_data = (mfxU8*)(payload + 1);
-- 
1.8.3.1

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

[FFmpeg-devel] [PATCH 2/3] lavf/mov: improve `tref/chap` chapter handling

[Rodger Combs]

3 parts:
- Supports multiple chapter streams
- Exports regular text chapter streams as opaque data. This prevents consumers
  from showing chapters as if they were regular subtitle streams.
- Exports video chapter streams as thumbnails, and provides the first one as
  an attached_pic.
---
 libavformat/isom.h |  3 ++-
 libavformat/mov.c  | 54 +++---
 2 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index 2246fed..9038057 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -210,7 +210,8 @@ typedef struct MOVContext {
 unsigned trex_count;
 int itunes_metadata;  ///< metadata are itunes style
 int handbrake_version;
-int chapter_track;
+int *chapter_tracks;
+unsigned int nb_chapter_tracks;
 int use_absolute_path;
 int ignore_editlist;
 int ignore_chapters;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index a15c8d1..d0f1316 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -3971,7 +3971,20 @@ static int mov_read_tfhd(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 
 static int mov_read_chap(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
-c->chapter_track = avio_rb32(pb);
+unsigned i, num;
+void *new_tracks;
+
+num = atom.size / 4;
+if (!(new_tracks = av_malloc_array(num, sizeof(int
+return AVERROR(ENOMEM);
+
+av_free(c->chapter_tracks);
+c->chapter_tracks = new_tracks;
+c->nb_chapter_tracks = num;
+
+for (i = 0; i < num && !pb->eof_reached; i++)
+c->chapter_tracks[i] = avio_rb32(pb);
+
 return 0;
 }
 
@@ -5034,25 +5047,50 @@ static int mov_probe(AVProbeData *p)
 static void mov_read_chapters(AVFormatContext *s)
 {
 MOVContext *mov = s->priv_data;
-AVStream *st = NULL;
+AVStream *st;
 MOVStreamContext *sc;
 int64_t cur_pos;
-int i;
+int i, j;
+int chapter_track;
 
+for (j = 0; j < mov->nb_chapter_tracks; j++) {
+chapter_track = mov->chapter_tracks[j];
+st = NULL;
 for (i = 0; i < s->nb_streams; i++)
-if (s->streams[i]->id == mov->chapter_track) {
+if (s->streams[i]->id == chapter_track) {
 st = s->streams[i];
 break;
 }
 if (!st) {
 av_log(s, AV_LOG_ERROR, "Referenced QT chapter track not found\n");
-return;
+continue;
 }
 
-st->discard = AVDISCARD_ALL;
 sc = st->priv_data;
 cur_pos = avio_tell(sc->pb);
 
+if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
+st->disposition |= AV_DISPOSITION_ATTACHED_PIC | 
AV_DISPOSITION_TIMED_THUMBNAILS;
+if (st->nb_index_entries) {
+// Retrieve the first frame, if possible
+AVPacket pkt;
+AVIndexEntry *sample = >index_entries[0];
+if (avio_seek(sc->pb, sample->pos, SEEK_SET) != sample->pos) {
+av_log(s, AV_LOG_ERROR, "Failed to retrieve first frame\n");
+goto finish;
+}
+
+if (av_get_packet(sc->pb, , sample->size) < 0)
+goto finish;
+
+st->attached_pic  = pkt;
+st->attached_pic.stream_index = st->index;
+st->attached_pic.flags   |= AV_PKT_FLAG_KEY;
+}
+} else {
+st->codecpar->codec_type = AVMEDIA_TYPE_DATA;
+st->codecpar->codec_id = AV_CODEC_ID_BIN_DATA;
+st->discard = AVDISCARD_ALL;
 for (i = 0; i < st->nb_index_entries; i++) {
 AVIndexEntry *sample = >index_entries[i];
 int64_t end = i+1 < st->nb_index_entries ? 
st->index_entries[i+1].timestamp : st->duration;
@@ -5101,8 +5139,10 @@ static void mov_read_chapters(AVFormatContext *s)
 avpriv_new_chapter(s, i, st->time_base, sample->timestamp, end, title);
 av_freep();
 }
+}
 finish:
 avio_seek(sc->pb, cur_pos, SEEK_SET);
+}
 }
 
 static int parse_timecode_in_framenum_format(AVFormatContext *s, AVStream *st,
@@ -5425,7 +5465,7 @@ static int mov_read_header(AVFormatContext *s)
 av_log(mov->fc, AV_LOG_TRACE, "on_parse_exit_offset=%"PRId64"\n", 
avio_tell(pb));
 
 if (pb->seekable) {
-if (mov->chapter_track > 0 && !mov->ignore_chapters)
+if (mov->nb_chapter_tracks > 0 && !mov->ignore_chapters)
 mov_read_chapters(s);
 for (i = 0; i < s->nb_streams; i++)
 if (s->streams[i]->codecpar->codec_tag == AV_RL32("tmcd")) {
-- 
2.10.0

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

[FFmpeg-devel] [PATCH 3/3] lavf/mov: reindent

[Rodger Combs]

Reviewed-By: Michael Niedermayer 
---
 libavformat/mov.c | 156 +++---
 1 file changed, 78 insertions(+), 78 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index d0f1316..c9fef8a 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -5054,94 +5054,94 @@ static void mov_read_chapters(AVFormatContext *s)
 int chapter_track;
 
 for (j = 0; j < mov->nb_chapter_tracks; j++) {
-chapter_track = mov->chapter_tracks[j];
-st = NULL;
-for (i = 0; i < s->nb_streams; i++)
-if (s->streams[i]->id == chapter_track) {
-st = s->streams[i];
-break;
+chapter_track = mov->chapter_tracks[j];
+st = NULL;
+for (i = 0; i < s->nb_streams; i++)
+if (s->streams[i]->id == chapter_track) {
+st = s->streams[i];
+break;
+}
+if (!st) {
+av_log(s, AV_LOG_ERROR, "Referenced QT chapter track not found\n");
+continue;
 }
-if (!st) {
-av_log(s, AV_LOG_ERROR, "Referenced QT chapter track not found\n");
-continue;
-}
 
-sc = st->priv_data;
-cur_pos = avio_tell(sc->pb);
+sc = st->priv_data;
+cur_pos = avio_tell(sc->pb);
+
+if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
+st->disposition |= AV_DISPOSITION_ATTACHED_PIC | 
AV_DISPOSITION_TIMED_THUMBNAILS;
+if (st->nb_index_entries) {
+// Retrieve the first frame, if possible
+AVPacket pkt;
+AVIndexEntry *sample = >index_entries[0];
+if (avio_seek(sc->pb, sample->pos, SEEK_SET) != sample->pos) {
+av_log(s, AV_LOG_ERROR, "Failed to retrieve first 
frame\n");
+goto finish;
+}
 
-if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
-st->disposition |= AV_DISPOSITION_ATTACHED_PIC | 
AV_DISPOSITION_TIMED_THUMBNAILS;
-if (st->nb_index_entries) {
-// Retrieve the first frame, if possible
-AVPacket pkt;
-AVIndexEntry *sample = >index_entries[0];
-if (avio_seek(sc->pb, sample->pos, SEEK_SET) != sample->pos) {
-av_log(s, AV_LOG_ERROR, "Failed to retrieve first frame\n");
-goto finish;
-}
+if (av_get_packet(sc->pb, , sample->size) < 0)
+goto finish;
 
-if (av_get_packet(sc->pb, , sample->size) < 0)
-goto finish;
+st->attached_pic  = pkt;
+st->attached_pic.stream_index = st->index;
+st->attached_pic.flags   |= AV_PKT_FLAG_KEY;
+}
+} else {
+st->codecpar->codec_type = AVMEDIA_TYPE_DATA;
+st->codecpar->codec_id = AV_CODEC_ID_BIN_DATA;
+st->discard = AVDISCARD_ALL;
+for (i = 0; i < st->nb_index_entries; i++) {
+AVIndexEntry *sample = >index_entries[i];
+int64_t end = i+1 < st->nb_index_entries ? 
st->index_entries[i+1].timestamp : st->duration;
+uint8_t *title;
+uint16_t ch;
+int len, title_len;
+
+if (end < sample->timestamp) {
+av_log(s, AV_LOG_WARNING, "ignoring stream duration which 
is shorter than chapters\n");
+end = AV_NOPTS_VALUE;
+}
 
-st->attached_pic  = pkt;
-st->attached_pic.stream_index = st->index;
-st->attached_pic.flags   |= AV_PKT_FLAG_KEY;
-}
-} else {
-st->codecpar->codec_type = AVMEDIA_TYPE_DATA;
-st->codecpar->codec_id = AV_CODEC_ID_BIN_DATA;
-st->discard = AVDISCARD_ALL;
-for (i = 0; i < st->nb_index_entries; i++) {
-AVIndexEntry *sample = >index_entries[i];
-int64_t end = i+1 < st->nb_index_entries ? 
st->index_entries[i+1].timestamp : st->duration;
-uint8_t *title;
-uint16_t ch;
-int len, title_len;
-
-if (end < sample->timestamp) {
-av_log(s, AV_LOG_WARNING, "ignoring stream duration which is 
shorter than chapters\n");
-end = AV_NOPTS_VALUE;
-}
+if (avio_seek(sc->pb, sample->pos, SEEK_SET) != sample->pos) {
+av_log(s, AV_LOG_ERROR, "Chapter %d not found in file\n", 
i);
+goto finish;
+}
 
-if (avio_seek(sc->pb, sample->pos, SEEK_SET) != sample->pos) {
-av_log(s, AV_LOG_ERROR, "Chapter %d not found in file\n", i);
-goto finish;
-}
+// the first two bytes are the length of the title
+len = avio_rb16(sc->pb);
+if (len > sample->size-2)
+continue;
+title_len = 2*len + 1;
+if (!(title = av_mallocz(title_len)))
+   

[FFmpeg-devel] [PATCH 1/3] lavf: add AV_DISPOSITION_TIMED_THUMBNAILS

[Rodger Combs]

Reviewed-By: Michael Niedermayer 
---
 ffprobe.c   |  1 +
 libavformat/avformat.h  | 12 +---
 tests/ref/fate/concat-demuxer-extended-lavf-mxf |  2 +-
 tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 |  2 +-
 tests/ref/fate/concat-demuxer-simple1-lavf-mxf  |  4 ++--
 tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10  |  4 ++--
 tests/ref/fate/concat-demuxer-simple2-lavf-ts   |  4 ++--
 tests/ref/fate/ffprobe_compact  |  6 +++---
 tests/ref/fate/ffprobe_csv  |  6 +++---
 tests/ref/fate/ffprobe_default  |  3 +++
 tests/ref/fate/ffprobe_flat |  3 +++
 tests/ref/fate/ffprobe_ini  |  3 +++
 tests/ref/fate/ffprobe_json |  9 ++---
 tests/ref/fate/ffprobe_xml  |  6 +++---
 14 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/ffprobe.c b/ffprobe.c
index bb3979c..49fb779 100644
--- a/ffprobe.c
+++ b/ffprobe.c
@@ -2370,6 +2370,7 @@ static int show_stream(WriterContext *w, AVFormatContext 
*fmt_ctx, int stream_id
 PRINT_DISPOSITION(VISUAL_IMPAIRED,  "visual_impaired");
 PRINT_DISPOSITION(CLEAN_EFFECTS,"clean_effects");
 PRINT_DISPOSITION(ATTACHED_PIC, "attached_pic");
+PRINT_DISPOSITION(TIMED_THUMBNAILS, "timed_thumbnails");
 writer_print_section_footer(w);
 }
 
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 057f8c5..310952a 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -840,11 +840,17 @@ typedef struct AVIndexEntry {
 #define AV_DISPOSITION_CLEAN_EFFECTS 0x0200  /**< stream without voice */
 /**
  * The stream is stored in the file as an attached picture/"cover art" (e.g.
- * APIC frame in ID3v2). The single packet associated with it will be returned
- * among the first few packets read from the file unless seeking takes place.
- * It can also be accessed at any time in AVStream.attached_pic.
+ * APIC frame in ID3v2). The first (usually only) packet associated with it
+ * will be returned among the first few packets read from the file unless
+ * seeking takes place. It can also be accessed at any time in
+ * AVStream.attached_pic.
  */
 #define AV_DISPOSITION_ATTACHED_PIC  0x0400
+/**
+ * The stream is sparse, and contains thumbnail images, often corresponding
+ * to chapter markers. Only ever used with AV_DISPOSITION_ATTACHED_PIC.
+ */
+#define AV_DISPOSITION_TIMED_THUMBNAILS  0x0800
 
 typedef struct AVStreamInternal AVStreamInternal;
 
diff --git a/tests/ref/fate/concat-demuxer-extended-lavf-mxf 
b/tests/ref/fate/concat-demuxer-extended-lavf-mxf
index f7905aa..46f2880 100644
--- a/tests/ref/fate/concat-demuxer-extended-lavf-mxf
+++ b/tests/ref/fate/concat-demuxer-extended-lavf-mxf
@@ -1 +1 @@
-21eb3a629ff504b55c93a66879a31362 
*tests/data/fate/concat-demuxer-extended-lavf-mxf.ffprobe
+c8a46035dbb8f2d8d8cc3cc910ee9a45 
*tests/data/fate/concat-demuxer-extended-lavf-mxf.ffprobe
diff --git a/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 
b/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
index 0c49f1f..d5cbb37 100644
--- a/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
+++ b/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
@@ -1 +1 @@
-67a03ad49f1bd17131f751313639b61e 
*tests/data/fate/concat-demuxer-extended-lavf-mxf_d10.ffprobe
+093085c2712c0825ee91d651f6293b3d 
*tests/data/fate/concat-demuxer-extended-lavf-mxf_d10.ffprobe
diff --git a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf 
b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf
index 6bba76a..27d6d14 100644
--- a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf
+++ b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf
@@ -120,5 +120,5 @@ 
audio|1|65280|1.36|65280|1.36|1920|0.04|N/A|N/A|3840|206848|K_|1
 Strings Metadata|8
 video|0|37|1.48|34|1.36|1|0.04|N/A|N/A|24786|211456|K_|1
 Strings Metadata|8
-0|mpeg2video|4|video|1/25|[0][0][0][0]|0x|352|288|0|0|1|1:1|11:9|yuv420p|8|tv|unknown|unknown|unknown|left|N/A|1|N/A|25/1|25/1|1/25|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|51|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001301
-1|pcm_s16le|unknown|audio|1/48000|[0][0][0][0]|0x|s16|48000|1|unknown|16|N/A|0/0|0/0|1/48000|0|0.00|N/A|N/A|768000|N/A|N/A|N/A|N/A|50|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001301
+0|mpeg2video|4|video|1/25|[0][0][0][0]|0x|352|288|0|0|1|1:1|11:9|yuv420p|8|tv|unknown|unknown|unknown|left|N/A|1|N/A|25/1|25/1|1/25|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|51|0|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001301

Re: [FFmpeg-devel] [PATCH 01/11] avformat/matroskaenc: don't reserve space for stream duration tags if the output is not seekable

[Michael Niedermayer]

On Tue, Oct 04, 2016 at 09:23:37PM -0300, James Almer wrote:
> On 10/4/2016 9:00 PM, Michael Niedermayer wrote:
> > On Mon, Oct 03, 2016 at 08:36:57PM -0300, James Almer wrote:
> >> The durations are never written in that situation.
> >>
> >> Signed-off-by: James Almer 
> >> ---
> >>  libavformat/matroskaenc.c| 2 +-
> >>  tests/fate/matroska.mak  | 2 +-
> >>  tests/fate/wavpack.mak   | 4 ++--
> >>  tests/ref/fate/binsub-mksenc | 2 +-
> >>  4 files changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> >> index 3eeb09b..32d5dcf 100644
> >> --- a/libavformat/matroskaenc.c
> >> +++ b/libavformat/matroskaenc.c
> >> @@ -1376,7 +1376,7 @@ static int mkv_write_tags(AVFormatContext *s)
> >>  if (ret < 0) return ret;
> >>  }
> >>  
> >> -if (!mkv->is_live) {
> >> +if (s->pb->seekable && !mkv->is_live) {
> >>  for (i = 0; i < s->nb_streams; i++) {
> >>  ebml_master tag_target;
> >>  ebml_master tag;
> > 
> > LGTM
> > 
> > 
> >> diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak
> >> index 8e4a1e8..36cc779 100644
> >> --- a/tests/fate/matroska.mak
> >> +++ b/tests/fate/matroska.mak
> >> @@ -4,6 +4,6 @@
> >>  FATE_MATROSKA-$(call DEMMUX, MATROSKA, MATROSKA) += fate-matroska-remux
> >>  fate-matroska-remux: CMD = md5 -i 
> >> $(TARGET_SAMPLES)/vp9-test-vectors/vp90-2-2pass-akiyo.webm -color_trc 4 
> >> -c:v copy -fflags +bitexact -strict -2 -f matroska
> >>  fate-matroska-remux: CMP = oneline
> >> -fate-matroska-remux: REF = f08b20b90f158a4de5a02a52c25596b9
> >> +fate-matroska-remux: REF = 1040692ffdfee2428954af79a7d5d155
> > 
> > off topic, but storing the output files on disk and printing some
> > richer information would be quite usefull to understand changes
> > 
> > thx
> 
> Having tests like this where a non seekable protocol is used comes in
> handy. Notice how in this set these md5 tests are unaffected by most
> changes precisely because the output AVIOContext is not seekable.
> I guess what could be done is duplicate the tests, using both md5
> protocol and framecrc output.

i dont think duplication would really be optimal,
i was more thinking about
storing the output with a "non seekable" file protocol and then
printing information about that file

thx

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

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


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

Re: [FFmpeg-devel] [PATCH] doc/developer: remove duplicate policies and fix error

[James Almer]

On 10/4/2016 7:20 PM, Michael Niedermayer wrote:
> On Tue, Oct 04, 2016 at 11:10:00PM +0100, Josh de Kock wrote:
>> Fixes regression as of ee72b6d1
>>
>> Signed-off-by: Josh de Kock 
>> ---
>> The irony of this patch is terrible, but oh well. Maybe we're missing a
>> 'everyone makes mistakes' policy.
>>
>>  doc/developer.texi | 18 ++
>>  1 file changed, 2 insertions(+), 16 deletions(-)
> 
> should be ok, this is removing duplicate entries

Pushed as it broke fate and things were becoming very red very fast.

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

Re: [FFmpeg-devel] [PATCH 01/11] avformat/matroskaenc: don't reserve space for stream duration tags if the output is not seekable

[James Almer]

On 10/4/2016 9:00 PM, Michael Niedermayer wrote:
> On Mon, Oct 03, 2016 at 08:36:57PM -0300, James Almer wrote:
>> The durations are never written in that situation.
>>
>> Signed-off-by: James Almer 
>> ---
>>  libavformat/matroskaenc.c| 2 +-
>>  tests/fate/matroska.mak  | 2 +-
>>  tests/fate/wavpack.mak   | 4 ++--
>>  tests/ref/fate/binsub-mksenc | 2 +-
>>  4 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index 3eeb09b..32d5dcf 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -1376,7 +1376,7 @@ static int mkv_write_tags(AVFormatContext *s)
>>  if (ret < 0) return ret;
>>  }
>>  
>> -if (!mkv->is_live) {
>> +if (s->pb->seekable && !mkv->is_live) {
>>  for (i = 0; i < s->nb_streams; i++) {
>>  ebml_master tag_target;
>>  ebml_master tag;
> 
> LGTM
> 
> 
>> diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak
>> index 8e4a1e8..36cc779 100644
>> --- a/tests/fate/matroska.mak
>> +++ b/tests/fate/matroska.mak
>> @@ -4,6 +4,6 @@
>>  FATE_MATROSKA-$(call DEMMUX, MATROSKA, MATROSKA) += fate-matroska-remux
>>  fate-matroska-remux: CMD = md5 -i 
>> $(TARGET_SAMPLES)/vp9-test-vectors/vp90-2-2pass-akiyo.webm -color_trc 4 -c:v 
>> copy -fflags +bitexact -strict -2 -f matroska
>>  fate-matroska-remux: CMP = oneline
>> -fate-matroska-remux: REF = f08b20b90f158a4de5a02a52c25596b9
>> +fate-matroska-remux: REF = 1040692ffdfee2428954af79a7d5d155
> 
> off topic, but storing the output files on disk and printing some
> richer information would be quite usefull to understand changes
> 
> thx

Having tests like this where a non seekable protocol is used comes in
handy. Notice how in this set these md5 tests are unaffected by most
changes precisely because the output AVIOContext is not seekable.
I guess what could be done is duplicate the tests, using both md5
protocol and framecrc output.

In any case, what happened in here is that all of them aren't writing
Duration tags with a never-updated Void element inside anymore.

Applied, thanks.

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

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

Re: [FFmpeg-devel] [PATCH 01/11] avformat/matroskaenc: don't reserve space for stream duration tags if the output is not seekable

[Michael Niedermayer]

On Mon, Oct 03, 2016 at 08:36:57PM -0300, James Almer wrote:
> The durations are never written in that situation.
> 
> Signed-off-by: James Almer 
> ---
>  libavformat/matroskaenc.c| 2 +-
>  tests/fate/matroska.mak  | 2 +-
>  tests/fate/wavpack.mak   | 4 ++--
>  tests/ref/fate/binsub-mksenc | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 3eeb09b..32d5dcf 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -1376,7 +1376,7 @@ static int mkv_write_tags(AVFormatContext *s)
>  if (ret < 0) return ret;
>  }
>  
> -if (!mkv->is_live) {
> +if (s->pb->seekable && !mkv->is_live) {
>  for (i = 0; i < s->nb_streams; i++) {
>  ebml_master tag_target;
>  ebml_master tag;

LGTM


> diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak
> index 8e4a1e8..36cc779 100644
> --- a/tests/fate/matroska.mak
> +++ b/tests/fate/matroska.mak
> @@ -4,6 +4,6 @@
>  FATE_MATROSKA-$(call DEMMUX, MATROSKA, MATROSKA) += fate-matroska-remux
>  fate-matroska-remux: CMD = md5 -i 
> $(TARGET_SAMPLES)/vp9-test-vectors/vp90-2-2pass-akiyo.webm -color_trc 4 -c:v 
> copy -fflags +bitexact -strict -2 -f matroska
>  fate-matroska-remux: CMP = oneline
> -fate-matroska-remux: REF = f08b20b90f158a4de5a02a52c25596b9
> +fate-matroska-remux: REF = 1040692ffdfee2428954af79a7d5d155

off topic, but storing the output files on disk and printing some
richer information would be quite usefull to understand changes

thx

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

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch


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

Re: [FFmpeg-devel] [PATCH 1/3] avformat/matroska: write FlagInterlaced element in WebM

[James Almer]

On 10/3/2016 1:21 PM, James Almer wrote:
> On 10/3/2016 12:27 PM, Dave Rice wrote:
>> Hi,
>>
>>> On Oct 2, 2016, at 7:14 PM, James Almer  wrote:
>>>
>>> On 9/27/2016 3:03 PM, James Almer wrote:
 It's listed as supported in both 
 https://www.webmproject.org/docs/container/
 and https://matroska.org/technical/specs/index.html

 Signed-off-by: James Almer 
 ---
 libavformat/matroskaenc.c | 41 +
 1 file changed, 21 insertions(+), 20 deletions(-)
>>>
>>> Ping.
>>
>> Untested but from a read through, the patch looks good. FlagInterlaced is 
>> supported in both Matroska and webM, whereas FieldOrder (a new field from 
>> the CELLAR work on Matroska) is only supported in Matroska. IMHO though I 
>> think that FlagInterlaced without information on FieldOrder is not so useful 
>> and that the webM project should consider adopting FieldOrder as well. I'd 
>> prefer to see a patch to webM to consider adding FieldOrder to the format 
>> rather than see FieldOrder removed from the webM muxer in FFmpeg.
> 
> FieldOrder is not being removed from the WebM muxer with this patch. It,
> alongside FlagInterlaced, was never written by it. Only by the Matroska
> muxer.
> This patch follows the current spec and adds FlagInterlaced to WebM by
> moving the mode check to only filter out FieldOrder instead of the two
> elements when targeting WebM.
> 
> If Google ever changes the spec to also support FieldOrder then adding
> it will be as simple as removing the mode check.
> 

Pushed, thanks.

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

Re: [FFmpeg-devel] [PATCH] doc/developer: remove duplicate policies and fix error

[Michael Niedermayer]

On Tue, Oct 04, 2016 at 11:10:00PM +0100, Josh de Kock wrote:
> Fixes regression as of ee72b6d1
> 
> Signed-off-by: Josh de Kock 
> ---
> The irony of this patch is terrible, but oh well. Maybe we're missing a
> 'everyone makes mistakes' policy.
> 
>  doc/developer.texi | 18 ++
>  1 file changed, 2 insertions(+), 16 deletions(-)

should be ok, this is removing duplicate entries


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

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


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

[FFmpeg-devel] [PATCH] doc/developer: remove duplicate policies and fix error

[Josh de Kock]

Fixes regression as of ee72b6d1

Signed-off-by: Josh de Kock 
---
The irony of this patch is terrible, but oh well. Maybe we're missing a
'everyone makes mistakes' policy.

 doc/developer.texi | 18 ++
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/doc/developer.texi b/doc/developer.texi
index 3fcfe86..cf809b9 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -293,12 +293,7 @@ later on.
 Also if you have doubts about splitting or not splitting, do not hesitate to
 ask/discuss it on the developer mailing list.
 
-@item API/ABI changes should be discussed before they are made.
-Do not change behavior of the programs (renaming options etc) or public
-API or ABI without first discussing it on the ffmpeg-devel mailing list.
-Do not remove widely used functionality or features (redundant code can be 
removed).
-
-@item Ask before you change the build system (configure, etc).
+@subheading Ask before you change the build system (configure, etc).
 Do not commit changes to the build system (Makefiles, configure script)
 which change behavior, defaults etc, without asking first. The same
 applies to compiler warning fixes, trivial looking fixes and to code
@@ -307,7 +302,7 @@ the way we do. Send your changes as patches to the 
ffmpeg-devel mailing
 list, and if the code maintainers say OK, you may commit. This does not
 apply to files you wrote and/or maintain.
 
-@item Cosmetic changes should be kept in separate patches.
+@subheading Cosmetic changes should be kept in separate patches.
 We refuse source indentation and other cosmetic changes if they are mixed
 with functional changes, such commits will be rejected and removed. Every
 developer has his own indentation style, you should not change it. Of course
@@ -356,15 +351,6 @@ Do not change behavior of the programs (renaming options 
etc) or public
 API or ABI without first discussing it on the ffmpeg-devel mailing list.
 Do not remove widely used functionality or features (redundant code can be 
removed).
 
-@subheading Ask before you change the build system (configure, etc).
-Do not commit changes to the build system (Makefiles, configure script)
-which change behavior, defaults etc, without asking first. The same
-applies to compiler warning fixes, trivial looking fixes and to code
-maintained by other developers. We usually have a reason for doing things
-the way we do. Send your changes as patches to the ffmpeg-devel mailing
-list, and if the code maintainers say OK, you may commit. This does not
-apply to files you wrote and/or maintain.
-
 @subheading Remember to check if you need to bump versions for libav*.
 Depending on the change, you may need to change the version integer.
 Incrementing the first component means no backward compatibility to
-- 
2.8.4 (Apple Git-73)

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

Re: [FFmpeg-devel] [PATCH] vf_fps: Don't flush a cached frame if it should have been dropped

[Derek Buitenhuis]

On 10/4/2016 10:21 PM, Michael Niedermayer wrote:
>> This "break" may actually be more correct output, but I am not familiar 
>> enough
>> with lavfi or vf_fps to say. Removing the last frame in case where it should 
>> have
>> been removed was the entire point of this patch.
> 
> the change to fate is wrong
> the input has a timebase of 1/3000, all timestamps are multiplies of
> 100 its thus basically 30 fps (with skiped frames) and the used
> -vf fps=30 should restore skip frames to make it cfr, loosing the last
> frame entirely is wrong in this case

I think you are correct. However, I'm not exactly sure of the proper way to fix
this. Not without more thought...

- Derek

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

Re: [FFmpeg-devel] [PATCH] vf_fps: Don't flush a cached frame if it should have been dropped

[Michael Niedermayer]

On Tue, Oct 04, 2016 at 10:13:41PM +0100, Derek Buitenhuis wrote:
> On 10/4/2016 9:58 PM, Michael Niedermayer wrote:
> > breaks fate
> > and the change to fate looks wrong, the last frame is lost
> > make fate-filter-fps
> > TESTfilter-fps
> > --- ./tests/ref/fate/filter-fps 2016-10-04 14:46:19.642736770 +0200
> > +++ tests/data/fate/filter-fps  2016-10-04 22:54:01.859353244 +0200
> > @@ -84,4 +84,3 @@
> >  0, 78, 78,1,30576, 0xa2fcd06f
> >  0, 79, 79,1,30576, 0xa2fcd06f
> >  0, 80, 80,1,30576, 0xa2fcd06f
> > -0, 81, 81,1,30576, 0xd4150aad
> > Test filter-fps failed. Look at tests/data/fate/filter-fps.err for details.
> 
> This "break" may actually be more correct output, but I am not familiar enough
> with lavfi or vf_fps to say. Removing the last frame in case where it should 
> have
> been removed was the entire point of this patch.

the change to fate is wrong
the input has a timebase of 1/3000, all timestamps are multiplies of
100 its thus basically 30 fps (with skiped frames) and the used
-vf fps=30 should restore skip frames to make it cfr, loosing the last
frame entirely is wrong in this case

codec copy:
#tb 0: 1/3000
...
0,  0,  0,  600, 4287, 0xa259fe7b
0,600,600,  700, 3951, 0x1bfc9daf, F=0x0
0,   1300,   1300, 6000, 4232, 0x75aeff18, F=0x0
0,   7300,   7300,  800, 4124, 0xc244436a, F=0x0
0,   8100,   8100,  695, 4104, 0x24aa4d34, F=0x0


without fps=30
0,  0,  0,1,30576, 0xcdc29b3d
0,  6,  6,1,30576, 0x5c83656c
0, 13, 13,1,30576, 0x26b67f83
0, 73, 73,1,30576, 0xa2fcd06f
0, 81, 81,1,30576, 0xd4150aad


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

Democracy is the form of government in which you can choose your dictator


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

Re: [FFmpeg-devel] [PATCH] vf_fps: Don't flush a cached frame if it should have been dropped

[Derek Buitenhuis]

On 10/4/2016 9:58 PM, Michael Niedermayer wrote:
> breaks fate
> and the change to fate looks wrong, the last frame is lost
> make fate-filter-fps
> TESTfilter-fps
> --- ./tests/ref/fate/filter-fps 2016-10-04 14:46:19.642736770 +0200
> +++ tests/data/fate/filter-fps  2016-10-04 22:54:01.859353244 +0200
> @@ -84,4 +84,3 @@
>  0, 78, 78,1,30576, 0xa2fcd06f
>  0, 79, 79,1,30576, 0xa2fcd06f
>  0, 80, 80,1,30576, 0xa2fcd06f
> -0, 81, 81,1,30576, 0xd4150aad
> Test filter-fps failed. Look at tests/data/fate/filter-fps.err for details.

This "break" may actually be more correct output, but I am not familiar enough
with lavfi or vf_fps to say. Removing the last frame in case where it should 
have
been removed was the entire point of this patch.

Perhaps someone more familiar with the code here can comment? Nicholas, maybe?

- Derek

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

Re: [FFmpeg-devel] [PATCH] vf_fps: Don't flush a cached frame if it should have been dropped

[Michael Niedermayer]

On Tue, Oct 04, 2016 at 04:21:29PM -0400, Derek Buitenhuis wrote:
> This fixes downconverting framerates to multiples.
> 
> For example, prior to this patch, converting 900 frames at 60 fps
> to 30 fps would output 451 frames instead of the correct 450.
> 
> Signed-off-by: Derek Buitenhuis 
> ---
> DISCLAIMER: I don't know libavfilter very well, and I am not
> sure this is the correct fix to the problem. Comments definitely
> welcome.
> 
> Also, I would be happy if any replies could be CC'd to me.

breaks fate
and the change to fate looks wrong, the last frame is lost
make fate-filter-fps
TESTfilter-fps
--- ./tests/ref/fate/filter-fps 2016-10-04 14:46:19.642736770 +0200
+++ tests/data/fate/filter-fps  2016-10-04 22:54:01.859353244 +0200
@@ -84,4 +84,3 @@
 0, 78, 78,1,30576, 0xa2fcd06f
 0, 79, 79,1,30576, 0xa2fcd06f
 0, 80, 80,1,30576, 0xa2fcd06f
-0, 81, 81,1,30576, 0xd4150aad
Test filter-fps failed. Look at tests/data/fate/filter-fps.err for details.

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

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


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

[FFmpeg-devel] [PATCH] vf_fps: Don't flush a cached frame if it should have been dropped

[Derek Buitenhuis]

This fixes downconverting framerates to multiples.

For example, prior to this patch, converting 900 frames at 60 fps
to 30 fps would output 451 frames instead of the correct 450.

Signed-off-by: Derek Buitenhuis 
---
DISCLAIMER: I don't know libavfilter very well, and I am not
sure this is the correct fix to the problem. Comments definitely
welcome.

Also, I would be happy if any replies could be CC'd to me.

Thanks,
  Derek
---
 libavfilter/vf_fps.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/libavfilter/vf_fps.c b/libavfilter/vf_fps.c
index 20ccd79..b6df968 100644
--- a/libavfilter/vf_fps.c
+++ b/libavfilter/vf_fps.c
@@ -135,15 +135,21 @@ static int request_frame(AVFilterLink *outlink)
 int i;
 for (i = 0; av_fifo_size(s->fifo); i++) {
 AVFrame *buf;
+int64_t delta;
 
 av_fifo_generic_read(s->fifo, , sizeof(buf), NULL);
 buf->pts = av_rescale_q(s->first_pts, ctx->inputs[0]->time_base,
 outlink->time_base) + s->frames_out;
 
-if ((ret = ff_filter_frame(outlink, buf)) < 0)
-return ret;
+/* number of output frames */
+delta = av_rescale_q_rnd(buf->pts - s->first_pts, 
ctx->inputs[0]->time_base,
+ outlink->time_base, s->rounding) - 
s->frames_out ;
+if (delta >= 0) {
+if ((ret = ff_filter_frame(outlink, buf)) < 0)
+return ret;
 
-s->frames_out++;
+s->frames_out++;
+}
 }
 return 0;
 }
-- 
1.8.3.1

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

Re: [FFmpeg-devel] [PATCH 0/4] V12 - SCTE-35 support

[Carlos Fernandez Sanz]

On Tue, Oct 4, 2016 at 12:11 PM, Josh de Kock  wrote:

> Putting SCTE35 support in another library like Upipe was suggested as well.

This is something that I don't understand. Should FFmpeg just ignore
the existance of this very important standard in the professional
video world? Drive users to something else, even though precisely
these professional users often sponsor development?

> My other question would be, if it does happen to get merged, how can I
> disable SCTE35 entirely?

Two questions
1) Does my patch break anything? If yes - it's just not ready to be merged.
2) SCTE-35, by its very definition, it's to cue video rebroadcasters
to switch from one stream to another in real time, for example to add
local commercials during a break. It's not a consumer format and you
are just not going to see SCTE-35 in any stream "in the wild". By the
time the stream reaches the consumer SCTE-35 has already done its job.
However if you do deal with a stream that contains SCTE-35 data most
likely you don't want to ignore it.

Or maybe I'm just missing some real world scenarios?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 0/4] V12 - SCTE-35 support

[Josh de Kock]

On 01/10/2016 18:27, Carlos Fernandez Sanz wrote:

- Addresses new comments such as like idx value checking and filename checking

Carlos Fernandez (4):
  Adding SCTE-35 CUI codec
  SCTE-35 extraction from mpegts
  SCTE-35 support in hlsenc
  Correct Indentation

 libavcodec/avcodec.h|   2 +
 libavcodec/codec_desc.c |   6 +
 libavformat/Makefile|   2 +-
 libavformat/hlsenc.c| 120 ---
 libavformat/mpegts.c|  48 -
 libavformat/scte_35.c   | 527 
 libavformat/scte_35.h   |  86 
 7 files changed, 764 insertions(+), 27 deletions(-)
 create mode 100644 libavformat/scte_35.c
 create mode 100644 libavformat/scte_35.h



There's been a fair amount of opposition to this set by a few other 
developers. The main concerns were regarding the timestamps,and how in 
STCE35 the PTS/DTS can represent both future video packets and others 
which signify the beginning of the commercial.


The other thing which was fairly ambiguous was how it would work with 
FFmpeg's timestamps due to how the AVPacket would not contain valid 
timestamps (how they'd only be in the data). There may be any number of 
timestamp values within in the AVPacket data stream, and while this may 
work for one stream, if you add more than one stream there would be no 
way to sync streams.


Putting SCTE35 support in another library like Upipe was suggested as 
well. My other question would be, if it does happen to get merged, how 
can I disable SCTE35 entirely?


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

Re: [FFmpeg-devel] [PATCH] lavc: set best effort timestamp if unset when using new decode API

[Michael Niedermayer]

On Tue, Oct 04, 2016 at 06:33:48PM +0200, wm4 wrote:
> Some API users (in particular ffmpeg.c) check the best effort timestamp
> only.
> ---
> Using guess_correct_pts() - not sure what I was thinking.
> ---
>  libavcodec/utils.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)

LGTM

thx

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

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


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

Re: [FFmpeg-devel] [PATCH 2/4] V12 - SCTE-35 extraction from mpegts

[Marton Balint]

On Mon, 3 Oct 2016, Carlos Fernandez Sanz wrote:


On Sat, Oct 1, 2016 at 10:50 AM, Marton Balint  wrote:


Empty line.


Thanks, corrected in V13.

If all OK, can this patchset be merged?


I guess patch 1 and 2 are OK now. However, I am not sure the third 
(hls) patch got a proper review, maybe you should ping the HLS maintainer Anssi 
Hannula.


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

Re: [FFmpeg-devel] [PATCH v2 1/2] doc/developer: reword some of the policies

[Josh de Kock]

On 03/10/2016 00:05, Michael Niedermayer wrote:

On Sun, Oct 02, 2016 at 11:16:49PM +0100, Josh de Kock wrote:



On 02/10/2016 22:47, Michael Niedermayer wrote:

On Sun, Oct 02, 2016 at 01:51:41AM +0100, Josh de Kock wrote:

Explicitly state that FATE should pass, and code should work
for all reviewers who tested.

[...]

-@item
-Do not commit unrelated changes together, split them into self-contained
-pieces. Also do not forget that if part B depends on part A, but A does not
-depend on B, then A can and should be committed first and separate from B.
-Keeping changes well split into self-contained parts makes reviewing and
-understanding them on the commit log mailing list easier. This also helps
-in case of debugging later on.
+@item Testing must be adequate but not excessive.
+If it works for you, others, and passes FATE then it should be OK to commit
+it, provided it fits the other committing criteria. You should not worry about
+over-testing things. If your code has problems (portability, triggers
+compiler bugs, unusual environment etc) they will be reported and eventually
+fixed.
+
+@item Do not commit unrelated changes together.
+They should be split them into self-contained pieces. Also do not forget
+that if part B depends on part A, but A does not depend on B, then A can
+and should be committed first and separate from B. Keeping changes well
+split into self-contained parts makes reviewing and understanding them on
+the commit log mailing list easier. This also helps in case of debugging
+later on.
Also if you have doubts about splitting or not splitting, do not hesitate to
ask/discuss it on the developer mailing list.




-@item
+@item API/ABI breakages and changes should be discussed before they are made.


I dont think this should list "breakages"
"breakages" are a subset of "changes"
and except in exteemly rare cases "breakages" should not happen
intentionally



That makes sense, I'll push a couple days with `s/breakages and //`
if there are no further comments.


not sure this fits in the patchset but
maybe "deprecation" should be added as deprecation implies future
change. Such change of course needs to be discussed. Discussing it
at the time of deprecation is better than after.



Applied without the deprecation addition.

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

Re: [FFmpeg-devel] [PATCH 2/2] libavcodec/ivi_dsp.c: fix warning due to misleading indentation

[Josh de Kock]

On 03/10/2016 15:40, Josh de Kock wrote:

On 02/10/2016 19:46, Adriano Pallavicino wrote:

LGTM. Will apply both patches squashed in a day if there are no further
comments.



Applied.

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

Re: [FFmpeg-devel] [PATCH 1/4] V13 - Adding SCTE-35 CUI codec

[Marton Balint]

On Mon, 3 Oct 2016, Carlos Fernandez Sanz wrote:


From: Carlos Fernandez 

Signed-off-by: Carlos Fernandez 
---
libavcodec/avcodec.h| 2 ++
libavcodec/codec_desc.c | 6 ++
2 files changed, 8 insertions(+)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index d72ee07..9064006 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -631,6 +631,8 @@ enum AVCodecID {
AV_CODEC_ID_FIRST_UNKNOWN = 0x18000,   ///< A dummy ID pointing at 
the start of various fake codecs.
AV_CODEC_ID_TTF = 0x18000,

+AV_CODEC_ID_SCTE_35,/**< Contain no valid time stamp in DTS PTS of 
avpacket, avpacket data contain time stamp
+  in scte-35 format which is relative to DTS/PTS 
of video stream */


Is this text still correct? I thought packet timestamps are estimated from 
the PCR of the program the scte stream is part of, which are relative to 
the SCTE timestamps in the data part?


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

[FFmpeg-devel] [PATCH] lavc: set best effort timestamp if unset when using new decode API

[wm4]

Some API users (in particular ffmpeg.c) check the best effort timestamp
only.
---
Using guess_correct_pts() - not sure what I was thinking.
---
 libavcodec/utils.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index ef3da65..9f8f9c7 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -2867,7 +2867,14 @@ int attribute_align_arg 
avcodec_receive_frame(AVCodecContext *avctx, AVFrame *fr
 if (avctx->codec->receive_frame) {
 if (avctx->internal->draining && !(avctx->codec->capabilities & 
AV_CODEC_CAP_DELAY))
 return AVERROR_EOF;
-return avctx->codec->receive_frame(avctx, frame);
+ret = avctx->codec->receive_frame(avctx, frame);
+if (ret >= 0) {
+if (av_frame_get_best_effort_timestamp(frame) == AV_NOPTS_VALUE) {
+av_frame_set_best_effort_timestamp(frame,
+guess_correct_pts(avctx, frame->pkt_pts, frame->pkt_dts));
+}
+}
+return ret;
 }
 
 // Emulation via old API.
-- 
2.9.3

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

Re: [FFmpeg-devel] [PATCH 3/3] lavf/mp3dec: read encoder delay/padding from Info tag

[James Almer]

On 10/4/2016 12:52 PM, Hendrik Leppkes wrote:
> On Tue, Oct 4, 2016 at 2:45 AM, Jon Toohill
>  wrote:
>> Muxers can check AVCodecParameters.initial_padding for the
>> encoder+decoder delay, and read the AV_PKT_DATA_SKIP_SAMPLES
>> side data from the last packet for the encoder padding.
>>
>> This change also fixes the first_discard_sample calculation
>> which erroneously included the decoder delay. Decoder delay
>> is already accounted for in st->skip_samples. The affected
>> FATE tests have been updated accordingly.
>> ---
>>  libavformat/mp3dec.c |  3 ++-
>>  tests/ref/fate/audiomatch-square-mp3 |  2 +-
>>  tests/ref/fate/gapless-mp3   | 10 +-
>>  3 files changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
>> index 56c7f8c..e8b2428 100644
>> --- a/libavformat/mp3dec.c
>> +++ b/libavformat/mp3dec.c
>> @@ -239,9 +239,10 @@ static void mp3_parse_info_tag(AVFormatContext *s, 
>> AVStream *st,
>>
>>  mp3->start_pad = v>>12;
>>  mp3->  end_pad = v&4095;
>> +st->codecpar->initial_padding = mp3->start_pad + 528 + 1;
>>  st->start_skip_samples = mp3->start_pad + 528 + 1;
>>  if (mp3->frames) {
>> -st->first_discard_sample = -mp3->end_pad + 528 + 1 + 
>> mp3->frames * (int64_t)spf;
>> +st->first_discard_sample = -mp3->end_pad + mp3->frames * 
>> (int64_t)spf;
>>  st->last_discard_sample = mp3->frames * (int64_t)spf;
>>  }
>>  if (!st->start_time)
>> diff --git a/tests/ref/fate/audiomatch-square-mp3 
>> b/tests/ref/fate/audiomatch-square-mp3
>> index 8de55c2..05176a0 100644
>> --- a/tests/ref/fate/audiomatch-square-mp3
>> +++ b/tests/ref/fate/audiomatch-square-mp3
>> @@ -1 +1 @@
>> -presig: 0 postsig:0 c: 0.9447 lenerr:0
>> +presig: 0 postsig:-529 c: 0.9334 lenerr:-529
>> diff --git a/tests/ref/fate/gapless-mp3 b/tests/ref/fate/gapless-mp3
>> index ebe7bfa..8b80bfc 100644
>> --- a/tests/ref/fate/gapless-mp3
>> +++ b/tests/ref/fate/gapless-mp3
>> @@ -1,5 +1,5 @@
>> -37534a3bcc3ef306e8c5ebfcfedfc41c *tests/data/fate/gapless-mp3.out-1
>> -c96c3ae7bd3300fd2f4debac222de5b7
>> -0cd1cdbcfd5cdbf6270cd98219bf31cd *tests/data/fate/gapless-mp3.out-2
>> -c96c3ae7bd3300fd2f4debac222de5b7
>> -9d3d8ba8a61b534f2d02ee648d6a8229 *tests/data/fate/gapless-mp3.out-3
>> +81695be427d45e8be4d527a6b2af2a85 *tests/data/fate/gapless-mp3.out-1
>> +c7879a827ab017364774069268d9a267
>> +62d074296f8c84a5f86a6afdd7bab459 *tests/data/fate/gapless-mp3.out-2
>> +c7879a827ab017364774069268d9a267
>> +e931f3fe1ba25e0d5eece4977c4061a9 *tests/data/fate/gapless-mp3.out-3
>> --
> 
> Presumably when these tests were setup, someone verified that their
> output was sane and proper, and gapless.
> So when these are changed, I have to ask - what exactly changes in
> this output? The hashes unfortunately don't tell us that.
> 
> - Hendrik

Changing the test to output the framecrc directly instead of doing a
md5sum of the output file shows this

TESTgapless-mp3
--- /ffmpeg/src/tests/ref/fate/gapless-mp3  2016-10-04 13:08:51.271126400 
-0300
+++ tests/data/fate/gapless-mp3 2016-10-04 13:09:26.519070600 -0300
@@ -596,7 +596,7 @@
 0,  217143996,  217143996,   368640,  418, 0xb260c6a6
 0,  217512636,  217512636,   368640,  418, 0xe448c368
 0,  217881276,  217881276,   368640,  418, 0xb229c63f
-0,  218249916,  218249916,   368640,  418, 0x53de9611, S=1,   10, 
0x011f0030
+0,  218249916,  218249916,   368640,  418, 0x53de9611, S=1,   10, 
0x018f0043
 0,  218618556,  218618556,   368640,  418, 0xe12f8514, S=1,   10, 
0x03140084
 #tb 0: 1/44100
 #media_type 0: audio
@@ -1196,7 +1196,7 @@
 0, 678575, 678575, 1152, 4608, 0x5fd9abc4
 0, 679727, 679727, 1152, 4608, 0xc7ccda46
 0, 680879, 680879, 1152, 4608, 0x96c68e2f
-0, 682031, 682031,  849, 3396, 0x4fe14cc5
+0, 682031, 682031,  320, 1280, 0xb3535bc6
 #tb 0: 1/14112000
 #media_type 0: audio
 #codec_id 0: mp3
@@ -1795,7 +1795,7 @@
 0,  217497600,  217497600,   368640,  418, 0xb260c6a6
 0,  217866240,  217866240,   368640,  418, 0xe448c368
 0,  218234880,  218234880,   368640,  418, 0xb229c63f
-0,  218603520,  218603520,   368640,  418, 0x53de9611, S=1,   10, 
0x011f0030
+0,  218603520,  218603520,   368640,  418, 0x53de9611, S=1,   10, 
0x018f0043
 0,  218972160,  218972160,   368640,  418, 0xe12f8514, S=1,   10, 
0x03140084
 #tb 0: 1/44100
 #media_type 0: audio
@@ -2395,7 +2395,7 @@
 0, 679680, 679680, 1152, 4608, 0x5fd9abc4
 0, 680832, 680832, 1152, 4608, 0xc7ccda46
 0, 681984, 681984, 1152, 4608, 0x96c68e2f
-0, 683136, 683136,  849, 3396, 0x4fe14cc5
+0, 683136, 683136,  320, 1280, 0xb3535bc6
 #tb 0: 1/14112000
 #media_type 0: audio
 #codec_id 0: mp3
@@ -2803,5 +2803,5 @@
 0,  

Re: [FFmpeg-devel] [PATCH] lavc: set best effort timestamp if unset when using new decode API

[Michael Niedermayer]

On Sun, Oct 02, 2016 at 06:56:48PM +0200, wm4 wrote:
> Some API users (in particular ffmpeg.c) check the best effort timestamp
> only.
> ---
> Still undecided if this is the right approach.
> ---
>  libavcodec/utils.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index ef3da65..1875a69 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -2867,7 +2867,12 @@ int attribute_align_arg 
> avcodec_receive_frame(AVCodecContext *avctx, AVFrame *fr
>  if (avctx->codec->receive_frame) {
>  if (avctx->internal->draining && !(avctx->codec->capabilities & 
> AV_CODEC_CAP_DELAY))
>  return AVERROR_EOF;
> -return avctx->codec->receive_frame(avctx, frame);
> +ret = avctx->codec->receive_frame(avctx, frame);
> +if (ret >= 0) {
> +if (av_frame_get_best_effort_timestamp(frame) == AV_NOPTS_VALUE)
> +av_frame_set_best_effort_timestamp(frame, frame->pkt_pts);

i think the pkt_dts should be considered as well if available
as in/with guess_correct_pts()

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

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad


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

Re: [FFmpeg-devel] [PATCH 3/3] lavf/mp3dec: read encoder delay/padding from Info tag

[Michael Niedermayer]

On Mon, Oct 03, 2016 at 05:45:08PM -0700, Jon Toohill wrote:
> Muxers can check AVCodecParameters.initial_padding for the
> encoder+decoder delay, and read the AV_PKT_DATA_SKIP_SAMPLES
> side data from the last packet for the encoder padding.
> 
> This change also fixes the first_discard_sample calculation
> which erroneously included the decoder delay. Decoder delay
> is already accounted for in st->skip_samples. The affected
> FATE tests have been updated accordingly.
> ---
>  libavformat/mp3dec.c |  3 ++-
>  tests/ref/fate/audiomatch-square-mp3 |  2 +-
>  tests/ref/fate/gapless-mp3   | 10 +-
>  3 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> index 56c7f8c..e8b2428 100644
> --- a/libavformat/mp3dec.c
> +++ b/libavformat/mp3dec.c
> @@ -239,9 +239,10 @@ static void mp3_parse_info_tag(AVFormatContext *s, 
> AVStream *st,
>  
>  mp3->start_pad = v>>12;
>  mp3->  end_pad = v&4095;
> +st->codecpar->initial_padding = mp3->start_pad + 528 + 1;
>  st->start_skip_samples = mp3->start_pad + 528 + 1;
>  if (mp3->frames) {
> -st->first_discard_sample = -mp3->end_pad + 528 + 1 + mp3->frames 
> * (int64_t)spf;
> +st->first_discard_sample = -mp3->end_pad + mp3->frames * 
> (int64_t)spf;
>  st->last_discard_sample = mp3->frames * (int64_t)spf;
>  }
>  if (!st->start_time)
> diff --git a/tests/ref/fate/audiomatch-square-mp3 
> b/tests/ref/fate/audiomatch-square-mp3
> index 8de55c2..05176a0 100644
> --- a/tests/ref/fate/audiomatch-square-mp3
> +++ b/tests/ref/fate/audiomatch-square-mp3
> @@ -1 +1 @@
> -presig: 0 postsig:0 c: 0.9447 lenerr:0
> +presig: 0 postsig:-529 c: 0.9334 lenerr:-529

isnt this exactly correct before and wrong after this change ?

zero signal before and zero signal after the original is what one would
expect and equal length and high correlation

every number that changes gets worse

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Never trust a computer, one day, it may think you are the virus. -- Compn


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

Re: [FFmpeg-devel] [PATCH]configure: Enable pie for toolchain=hardened.

[Michael Niedermayer]

On Tue, Oct 04, 2016 at 12:24:00PM +0200, Carl Eugen Hoyos wrote:
> Hi!
> 
> Sorry if I miss something but with this patch, the hardening_check 
> script succeeds here both for x86_32 and x86_64 (static and shared).
> 
> Please comment, Carl Eugen

only case i found that breaks is with --enable-libxavs
/usr/bin/ld: /usr/local/lib/libxavs.a(common.o): relocation R_X86_64_32 against 
`.rodata.str1.1' can not be used when making a shared object; recompile with 
-fPIC
/usr/local/lib/libxavs.a: could not read symbols: Bad value

not sure our configure should detect this or if thats beyond its
scope

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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

Re: [FFmpeg-devel] [PATCH] d3d11va: use the proper slice index

[James Almer]

On 10/4/2016 11:47 AM, Steve Lhomme wrote:
> The slice index expected by D3D11VA is the one from the texture not from the
> array or texture/slices.
> 
> In VLC the slices we provide the decoder don't start from 0 and thus pictures
> appear in bogus order. With possible crashes and corruptions when using an
> invalid index.
> ---
>  libavcodec/dxva2.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/libavcodec/dxva2.c b/libavcodec/dxva2.c
> index f68df86..f8801c9 100644
> --- a/libavcodec/dxva2.c
> +++ b/libavcodec/dxva2.c
> @@ -43,7 +43,16 @@ unsigned ff_dxva2_get_surface_index(const AVCodecContext 
> *avctx,
>  
>  for (i = 0; i < DXVA_CONTEXT_COUNT(avctx, ctx); i++)
>  if (DXVA_CONTEXT_SURFACE(avctx, ctx, i) == surface)
> +#if CONFIG_D3D11VA
> +{
> +ID3D11VideoDecoderOutputView *pOut = DXVA_CONTEXT_SURFACE(avctx, 
> ctx, i);
> +D3D11_VIDEO_DECODER_OUTPUT_VIEW_DESC viewDesc;
> +ID3D11VideoDecoderOutputView_GetDesc( pOut,  );
> +return viewDesc.Texture2D.ArraySlice;
> +}
> +#elif CONFIG_DXVA2

This will break DXVA2 if D3D11 is also enabled.
The rest of the file supports both cases by doing something like

#if CONFIG_D3D11VA
if (avctx->pix_fmt == AV_PIX_FMT_D3D11VA_VLD) {
...
}
#endif
#if CONFIG_DXVA2
if (avctx->pix_fmt == AV_PIX_FMT_DXVA2_VLD) {
...
}
#endif

So the same should probably be done here.
You could also simplify all this by replacing the DXVA_CONTEXT_SURFACE macro,
which already does a avctx->pix_fmt check, with the corresponding contexts
for D3D11 and DXVA2.

>  return i;
> +#endif
>  
>  assert(0);
>  return 0;
> 

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

Re: [FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check

[James Almer]

On 10/4/2016 11:35 AM, Hendrik Leppkes wrote:
> On Tue, Oct 4, 2016 at 4:32 PM, wm4  wrote:
>> On Tue, 4 Oct 2016 14:15:03 +0200
>> Michael Niedermayer  wrote:
>>
>>> On Tue, Oct 04, 2016 at 01:52:02PM +0200, Hendrik Leppkes wrote:
 On Tue, Oct 4, 2016 at 1:44 PM, Hendrik Leppkes  
 wrote:
> On Tue, Oct 4, 2016 at 1:23 PM, Michael Niedermayer
>  wrote:
>> On Tue, Oct 04, 2016 at 08:41:42AM +0200, Hendrik Leppkes wrote:
>>> On Tue, Oct 4, 2016 at 4:05 AM, Michael Niedermayer
>>>  wrote:
 On Sat, Oct 01, 2016 at 04:15:45PM +0200, Hendrik Leppkes wrote:
> Decoders have previously not used AVFrame.pts, and with the upcoming
> deprecation of pkt_pts (in favor of pts), this would lead to an 
> errorneous
> interpration of timestamps.

 I probably misunderstand the commit message but
 If code is changed in a user application that cannot really lift
 some blockage from changing a lib.
 a lib can only change in an incompaible way with (deprecation and)
 major version bump.

>>>
>>> The lib never did what this code suggests it did, not that I remember
>>> (so at least not for a long long time).
>>
>> release/2.0 with
>>
>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> index 29d5492..57c8d50 100644
>> --- a/libavcodec/utils.c
>> +++ b/libavcodec/utils.c
>> @@ -2008,7 +2008,7 @@ int attribute_align_arg 
>> avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
>>  avci->to_free.extended_data = avci->to_free.data;
>>  memset(picture->buf, 0, sizeof(picture->buf));
>>  }
>> -
>> +av_assert0(picture->pts == 0 || picture->pts == AV_NOPTS_VALUE);
>>  avctx->frame_number++;
>>  av_frame_set_best_effort_timestamp(picture,
>> guess_correct_pts(avctx,
>>
>> causes many tests to fail, indicating that AVFrame.pts was set for
>> several video decoders, the ffmpeg code is audio decoder specific
>> and i failed to find a case where it was triggered, i tried IIRC 3
>> or so checkouts from the past
>>
>> so AVFrame.pts was maybe never set for decoding audio but it was set
>> for video
>
> Can you extend the test to add "|| picture->pts == picture->pkt_pts"?
> Because thats what it would be set to after the merge. A quick check
> in the 2.0 code base looks like some decoders did set that, but to the
> exact same value as pkt_pts (which is what the merge is proposing
> right now as well)
>

 And I found this (after 2.0):
 http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=a1c5cc429d99216406170eac7e8352860076d3e8

 Which apparently set pts for mpeg4 to a number parsed from the
 bitstream, entirely uncorrelated to container or audio timestamps, so
 using them would have been rather impractical for any real use-cases.
>>>
>>> They can be usfull, some random examples:
>>>
>>> playing MPEG4-ES with timing stored from the bitstream (assuming there
>>>   is no demuxer lib used that is capable to extract them)
>>>
>>> forensics, raw video stream could have its timing
>>>   recovered, a video with manipulated container timestamps could be
>>>   detected.
>>>
>>> error correction, if the container level timestamps are lost or
>>>   corrupted the stream level ones can be used to recreate them
>>>
>>> There may be more, these are just some of the top of my head,
>>> not your mainstream multimedia player stuff maybe but they arent
>>> useless
>>>
>>> [...]
>>>
>>
>> They don't belong into the AVFrame.pts field, though.
> 
> And they don't go in there anymore right now, so thats that.
> 
> The real question is, what do we do about this merge now?
> Can we set AVFrame.pts to the same value as AVFrame.pkt_pts safely,
> considering it was unused in the current ABI/API, or would that be
> considered an API break and we better delay this change until the next
> major?
> 
> - Hendrik

Delaying it could result in further merges becoming technically wrong,
or at least require extra manual changes for them to not misbehave in
our tree.

IMO merge it now, and if needed/preferred, we could make sure it
doesn't make it to 3.2

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

[FFmpeg-devel] [PATCH] d3d11va: use the proper slice index

[Steve Lhomme]

The slice index expected by D3D11VA is the one from the texture not from the
array or texture/slices.

In VLC the slices we provide the decoder don't start from 0 and thus pictures
appear in bogus order. With possible crashes and corruptions when using an
invalid index.
---
 libavcodec/dxva2.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/libavcodec/dxva2.c b/libavcodec/dxva2.c
index f68df86..f8801c9 100644
--- a/libavcodec/dxva2.c
+++ b/libavcodec/dxva2.c
@@ -43,7 +43,16 @@ unsigned ff_dxva2_get_surface_index(const AVCodecContext 
*avctx,
 
 for (i = 0; i < DXVA_CONTEXT_COUNT(avctx, ctx); i++)
 if (DXVA_CONTEXT_SURFACE(avctx, ctx, i) == surface)
+#if CONFIG_D3D11VA
+{
+ID3D11VideoDecoderOutputView *pOut = DXVA_CONTEXT_SURFACE(avctx, 
ctx, i);
+D3D11_VIDEO_DECODER_OUTPUT_VIEW_DESC viewDesc;
+ID3D11VideoDecoderOutputView_GetDesc( pOut,  );
+return viewDesc.Texture2D.ArraySlice;
+}
+#elif CONFIG_DXVA2
 return i;
+#endif
 
 assert(0);
 return 0;
-- 
2.8.2

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

Re: [FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check

[wm4]

On Tue, 4 Oct 2016 16:35:02 +0200
Hendrik Leppkes  wrote:

> On Tue, Oct 4, 2016 at 4:32 PM, wm4  wrote:
> > On Tue, 4 Oct 2016 14:15:03 +0200
> > Michael Niedermayer  wrote:
> >  
> >> On Tue, Oct 04, 2016 at 01:52:02PM +0200, Hendrik Leppkes wrote:  
> >> > On Tue, Oct 4, 2016 at 1:44 PM, Hendrik Leppkes  
> >> > wrote:  
> >> > > On Tue, Oct 4, 2016 at 1:23 PM, Michael Niedermayer
> >> > >  wrote:  
> >> > >> On Tue, Oct 04, 2016 at 08:41:42AM +0200, Hendrik Leppkes wrote:  
> >> > >>> On Tue, Oct 4, 2016 at 4:05 AM, Michael Niedermayer
> >> > >>>  wrote:  
> >> > >>> > On Sat, Oct 01, 2016 at 04:15:45PM +0200, Hendrik Leppkes wrote:  
> >> > >>> >> Decoders have previously not used AVFrame.pts, and with the 
> >> > >>> >> upcoming
> >> > >>> >> deprecation of pkt_pts (in favor of pts), this would lead to an 
> >> > >>> >> errorneous
> >> > >>> >> interpration of timestamps.  
> >> > >>> >
> >> > >>> > I probably misunderstand the commit message but
> >> > >>> > If code is changed in a user application that cannot really lift
> >> > >>> > some blockage from changing a lib.
> >> > >>> > a lib can only change in an incompaible way with (deprecation and)
> >> > >>> > major version bump.
> >> > >>> >  
> >> > >>>
> >> > >>> The lib never did what this code suggests it did, not that I remember
> >> > >>> (so at least not for a long long time).  
> >> > >>
> >> > >> release/2.0 with
> >> > >>
> >> > >> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> >> > >> index 29d5492..57c8d50 100644
> >> > >> --- a/libavcodec/utils.c
> >> > >> +++ b/libavcodec/utils.c
> >> > >> @@ -2008,7 +2008,7 @@ int attribute_align_arg 
> >> > >> avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
> >> > >>  avci->to_free.extended_data = avci->to_free.data;
> >> > >>  memset(picture->buf, 0, sizeof(picture->buf));
> >> > >>  }
> >> > >> -
> >> > >> +av_assert0(picture->pts == 0 || picture->pts == AV_NOPTS_VALUE);
> >> > >>  avctx->frame_number++;
> >> > >>  av_frame_set_best_effort_timestamp(picture,
> >> > >> 
> >> > >> guess_correct_pts(avctx,
> >> > >>
> >> > >> causes many tests to fail, indicating that AVFrame.pts was set for
> >> > >> several video decoders, the ffmpeg code is audio decoder specific
> >> > >> and i failed to find a case where it was triggered, i tried IIRC 3
> >> > >> or so checkouts from the past
> >> > >>
> >> > >> so AVFrame.pts was maybe never set for decoding audio but it was set
> >> > >> for video  
> >> > >
> >> > > Can you extend the test to add "|| picture->pts == picture->pkt_pts"?
> >> > > Because thats what it would be set to after the merge. A quick check
> >> > > in the 2.0 code base looks like some decoders did set that, but to the
> >> > > exact same value as pkt_pts (which is what the merge is proposing
> >> > > right now as well)
> >> > >  
> >> >
> >> > And I found this (after 2.0):
> >> > http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=a1c5cc429d99216406170eac7e8352860076d3e8
> >> >
> >> > Which apparently set pts for mpeg4 to a number parsed from the
> >> > bitstream, entirely uncorrelated to container or audio timestamps, so
> >> > using them would have been rather impractical for any real use-cases.  
> >>
> >> They can be usfull, some random examples:
> >>
> >> playing MPEG4-ES with timing stored from the bitstream (assuming there
> >>   is no demuxer lib used that is capable to extract them)
> >>
> >> forensics, raw video stream could have its timing
> >>   recovered, a video with manipulated container timestamps could be
> >>   detected.
> >>
> >> error correction, if the container level timestamps are lost or
> >>   corrupted the stream level ones can be used to recreate them
> >>
> >> There may be more, these are just some of the top of my head,
> >> not your mainstream multimedia player stuff maybe but they arent
> >> useless
> >>
> >> [...]
> >>  
> >
> > They don't belong into the AVFrame.pts field, though.  
> 
> And they don't go in there anymore right now, so thats that.
> 
> The real question is, what do we do about this merge now?
> Can we set AVFrame.pts to the same value as AVFrame.pkt_pts safely,
> considering it was unused in the current ABI/API, or would that be
> considered an API break and we better delay this change until the next
> major?

IMO applications which did this were pretty broken anyway, and we
should be able to get away with a simple version bump.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check

[Hendrik Leppkes]

On Tue, Oct 4, 2016 at 4:32 PM, wm4  wrote:
> On Tue, 4 Oct 2016 14:15:03 +0200
> Michael Niedermayer  wrote:
>
>> On Tue, Oct 04, 2016 at 01:52:02PM +0200, Hendrik Leppkes wrote:
>> > On Tue, Oct 4, 2016 at 1:44 PM, Hendrik Leppkes  
>> > wrote:
>> > > On Tue, Oct 4, 2016 at 1:23 PM, Michael Niedermayer
>> > >  wrote:
>> > >> On Tue, Oct 04, 2016 at 08:41:42AM +0200, Hendrik Leppkes wrote:
>> > >>> On Tue, Oct 4, 2016 at 4:05 AM, Michael Niedermayer
>> > >>>  wrote:
>> > >>> > On Sat, Oct 01, 2016 at 04:15:45PM +0200, Hendrik Leppkes wrote:
>> > >>> >> Decoders have previously not used AVFrame.pts, and with the upcoming
>> > >>> >> deprecation of pkt_pts (in favor of pts), this would lead to an 
>> > >>> >> errorneous
>> > >>> >> interpration of timestamps.
>> > >>> >
>> > >>> > I probably misunderstand the commit message but
>> > >>> > If code is changed in a user application that cannot really lift
>> > >>> > some blockage from changing a lib.
>> > >>> > a lib can only change in an incompaible way with (deprecation and)
>> > >>> > major version bump.
>> > >>> >
>> > >>>
>> > >>> The lib never did what this code suggests it did, not that I remember
>> > >>> (so at least not for a long long time).
>> > >>
>> > >> release/2.0 with
>> > >>
>> > >> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> > >> index 29d5492..57c8d50 100644
>> > >> --- a/libavcodec/utils.c
>> > >> +++ b/libavcodec/utils.c
>> > >> @@ -2008,7 +2008,7 @@ int attribute_align_arg 
>> > >> avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
>> > >>  avci->to_free.extended_data = avci->to_free.data;
>> > >>  memset(picture->buf, 0, sizeof(picture->buf));
>> > >>  }
>> > >> -
>> > >> +av_assert0(picture->pts == 0 || picture->pts == AV_NOPTS_VALUE);
>> > >>  avctx->frame_number++;
>> > >>  av_frame_set_best_effort_timestamp(picture,
>> > >> guess_correct_pts(avctx,
>> > >>
>> > >> causes many tests to fail, indicating that AVFrame.pts was set for
>> > >> several video decoders, the ffmpeg code is audio decoder specific
>> > >> and i failed to find a case where it was triggered, i tried IIRC 3
>> > >> or so checkouts from the past
>> > >>
>> > >> so AVFrame.pts was maybe never set for decoding audio but it was set
>> > >> for video
>> > >
>> > > Can you extend the test to add "|| picture->pts == picture->pkt_pts"?
>> > > Because thats what it would be set to after the merge. A quick check
>> > > in the 2.0 code base looks like some decoders did set that, but to the
>> > > exact same value as pkt_pts (which is what the merge is proposing
>> > > right now as well)
>> > >
>> >
>> > And I found this (after 2.0):
>> > http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=a1c5cc429d99216406170eac7e8352860076d3e8
>> >
>> > Which apparently set pts for mpeg4 to a number parsed from the
>> > bitstream, entirely uncorrelated to container or audio timestamps, so
>> > using them would have been rather impractical for any real use-cases.
>>
>> They can be usfull, some random examples:
>>
>> playing MPEG4-ES with timing stored from the bitstream (assuming there
>>   is no demuxer lib used that is capable to extract them)
>>
>> forensics, raw video stream could have its timing
>>   recovered, a video with manipulated container timestamps could be
>>   detected.
>>
>> error correction, if the container level timestamps are lost or
>>   corrupted the stream level ones can be used to recreate them
>>
>> There may be more, these are just some of the top of my head,
>> not your mainstream multimedia player stuff maybe but they arent
>> useless
>>
>> [...]
>>
>
> They don't belong into the AVFrame.pts field, though.

And they don't go in there anymore right now, so thats that.

The real question is, what do we do about this merge now?
Can we set AVFrame.pts to the same value as AVFrame.pkt_pts safely,
considering it was unused in the current ABI/API, or would that be
considered an API break and we better delay this change until the next
major?

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

Re: [FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check

[wm4]

On Tue, 4 Oct 2016 14:15:03 +0200
Michael Niedermayer  wrote:

> On Tue, Oct 04, 2016 at 01:52:02PM +0200, Hendrik Leppkes wrote:
> > On Tue, Oct 4, 2016 at 1:44 PM, Hendrik Leppkes  
> > wrote:  
> > > On Tue, Oct 4, 2016 at 1:23 PM, Michael Niedermayer
> > >  wrote:  
> > >> On Tue, Oct 04, 2016 at 08:41:42AM +0200, Hendrik Leppkes wrote:  
> > >>> On Tue, Oct 4, 2016 at 4:05 AM, Michael Niedermayer
> > >>>  wrote:  
> > >>> > On Sat, Oct 01, 2016 at 04:15:45PM +0200, Hendrik Leppkes wrote:  
> > >>> >> Decoders have previously not used AVFrame.pts, and with the upcoming
> > >>> >> deprecation of pkt_pts (in favor of pts), this would lead to an 
> > >>> >> errorneous
> > >>> >> interpration of timestamps.  
> > >>> >
> > >>> > I probably misunderstand the commit message but
> > >>> > If code is changed in a user application that cannot really lift
> > >>> > some blockage from changing a lib.
> > >>> > a lib can only change in an incompaible way with (deprecation and)
> > >>> > major version bump.
> > >>> >  
> > >>>
> > >>> The lib never did what this code suggests it did, not that I remember
> > >>> (so at least not for a long long time).  
> > >>
> > >> release/2.0 with
> > >>
> > >> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> > >> index 29d5492..57c8d50 100644
> > >> --- a/libavcodec/utils.c
> > >> +++ b/libavcodec/utils.c
> > >> @@ -2008,7 +2008,7 @@ int attribute_align_arg 
> > >> avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
> > >>  avci->to_free.extended_data = avci->to_free.data;
> > >>  memset(picture->buf, 0, sizeof(picture->buf));
> > >>  }
> > >> -
> > >> +av_assert0(picture->pts == 0 || picture->pts == AV_NOPTS_VALUE);
> > >>  avctx->frame_number++;
> > >>  av_frame_set_best_effort_timestamp(picture,
> > >> guess_correct_pts(avctx,
> > >>
> > >> causes many tests to fail, indicating that AVFrame.pts was set for
> > >> several video decoders, the ffmpeg code is audio decoder specific
> > >> and i failed to find a case where it was triggered, i tried IIRC 3
> > >> or so checkouts from the past
> > >>
> > >> so AVFrame.pts was maybe never set for decoding audio but it was set
> > >> for video  
> > >
> > > Can you extend the test to add "|| picture->pts == picture->pkt_pts"?
> > > Because thats what it would be set to after the merge. A quick check
> > > in the 2.0 code base looks like some decoders did set that, but to the
> > > exact same value as pkt_pts (which is what the merge is proposing
> > > right now as well)
> > >  
> > 
> > And I found this (after 2.0):
> > http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=a1c5cc429d99216406170eac7e8352860076d3e8
> > 
> > Which apparently set pts for mpeg4 to a number parsed from the
> > bitstream, entirely uncorrelated to container or audio timestamps, so
> > using them would have been rather impractical for any real use-cases.  
> 
> They can be usfull, some random examples:
> 
> playing MPEG4-ES with timing stored from the bitstream (assuming there
>   is no demuxer lib used that is capable to extract them)
> 
> forensics, raw video stream could have its timing
>   recovered, a video with manipulated container timestamps could be
>   detected.
> 
> error correction, if the container level timestamps are lost or
>   corrupted the stream level ones can be used to recreate them
> 
> There may be more, these are just some of the top of my head,
> not your mainstream multimedia player stuff maybe but they arent
> useless
> 
> [...]
> 

They don't belong into the AVFrame.pts field, though.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/3] lavf/mp3dec: read encoder delay/padding from Info tag

[wm4]

On Mon,  3 Oct 2016 17:45:08 -0700
Jon Toohill  wrote:

> Muxers can check AVCodecParameters.initial_padding for the
> encoder+decoder delay, and read the AV_PKT_DATA_SKIP_SAMPLES
> side data from the last packet for the encoder padding.
> 
> This change also fixes the first_discard_sample calculation
> which erroneously included the decoder delay. Decoder delay
> is already accounted for in st->skip_samples. The affected
> FATE tests have been updated accordingly.
> ---
>  libavformat/mp3dec.c |  3 ++-
>  tests/ref/fate/audiomatch-square-mp3 |  2 +-
>  tests/ref/fate/gapless-mp3   | 10 +-
>  3 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> index 56c7f8c..e8b2428 100644
> --- a/libavformat/mp3dec.c
> +++ b/libavformat/mp3dec.c
> @@ -239,9 +239,10 @@ static void mp3_parse_info_tag(AVFormatContext *s, 
> AVStream *st,
>  
>  mp3->start_pad = v>>12;
>  mp3->  end_pad = v&4095;
> +st->codecpar->initial_padding = mp3->start_pad + 528 + 1;
>  st->start_skip_samples = mp3->start_pad + 528 + 1;
>  if (mp3->frames) {
> -st->first_discard_sample = -mp3->end_pad + 528 + 1 + mp3->frames 
> * (int64_t)spf;
> +st->first_discard_sample = -mp3->end_pad + mp3->frames * 
> (int64_t)spf;

How does mixing these even make sense?

>  st->last_discard_sample = mp3->frames * (int64_t)spf;
>  }
>  if (!st->start_time)
> diff --git a/tests/ref/fate/audiomatch-square-mp3 
> b/tests/ref/fate/audiomatch-square-mp3
> index 8de55c2..05176a0 100644
> --- a/tests/ref/fate/audiomatch-square-mp3
> +++ b/tests/ref/fate/audiomatch-square-mp3
> @@ -1 +1 @@
> -presig: 0 postsig:0 c: 0.9447 lenerr:0
> +presig: 0 postsig:-529 c: 0.9334 lenerr:-529
> diff --git a/tests/ref/fate/gapless-mp3 b/tests/ref/fate/gapless-mp3
> index ebe7bfa..8b80bfc 100644
> --- a/tests/ref/fate/gapless-mp3
> +++ b/tests/ref/fate/gapless-mp3
> @@ -1,5 +1,5 @@
> -37534a3bcc3ef306e8c5ebfcfedfc41c *tests/data/fate/gapless-mp3.out-1
> -c96c3ae7bd3300fd2f4debac222de5b7
> -0cd1cdbcfd5cdbf6270cd98219bf31cd *tests/data/fate/gapless-mp3.out-2
> -c96c3ae7bd3300fd2f4debac222de5b7
> -9d3d8ba8a61b534f2d02ee648d6a8229 *tests/data/fate/gapless-mp3.out-3
> +81695be427d45e8be4d527a6b2af2a85 *tests/data/fate/gapless-mp3.out-1
> +c7879a827ab017364774069268d9a267
> +62d074296f8c84a5f86a6afdd7bab459 *tests/data/fate/gapless-mp3.out-2
> +c7879a827ab017364774069268d9a267
> +e931f3fe1ba25e0d5eece4977c4061a9 *tests/data/fate/gapless-mp3.out-3
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [GSoC] avcodec/als: Add ALS encoder

[Michael Niedermayer]

On Tue, Oct 04, 2016 at 12:22:38AM +0530, Umair Khan wrote:
> Hi,
> 
> Patch attached.
> 
> It fixes the fate tests.
> However, there's a slight bug in the encoder in handling the last frame.
> I'll definitely fix it later.
> I hope the patch can be merged in this state.
[...]
> +/**
> + * Quantize and rescale a single PARCOR coefficient.
> + * @param ctx Encoder context
> + * @param parcor double-precision PARCOR coefficient
> + * @param index  coefficient index number
> + * @param[out] q_parcor 7-bit quantized coefficient
> + * @param[out] r_parcor 21-bit reconstructed coefficient
> + * @return the number of bits used to encode the coefficient
> + */
> +static int quantize_single_parcor_coeff(ALSEncContext *ctx, double parcor,
> +int index, int32_t *q_parcor,
> +int32_t *r_parcor)
> +{
> +int rice_param, offset;
> +int sign = !index - index;
> +
> +// compand coefficient for index 0 or 1
> +if (index < 2)
> +parcor = sqrt(2.0 * (sign * parcor + 1.0)) - 1.0;
> +
> +// quantize to signed 7-bit

> +*q_parcor = av_clip((int32_t)floor(64.0 * parcor), -64, 63);

could use av_clip_intp2()


[...]
> +/**
> + * Determine the number of samples in each frame, which is constant for all
> + * frames in the stream except the very last one which may be smaller.
> + */
> +static void frame_partitioning(ALSEncContext *ctx)
> +{
> +AVCodecContext *avctx= ctx->avctx;
> +ALSSpecificConfig *sconf = >sconf;
> +
> +// choose default frame size if not specified by the user
> +if (avctx->frame_size <= 0) {
> +if (avctx->sample_rate <= 24000)
> +avctx->frame_size = 1024;
> +else if(avctx->sample_rate <= 48000)
> +avctx->frame_size = 2048;
> +else if(avctx->sample_rate <= 96000)
> +avctx->frame_size = 4096;
> +else
> +avctx->frame_size = 8192;
> +
> +// increase frame size if block switching is used
> +if (sconf->block_switching)
> +avctx->frame_size <<= sconf->block_switching >> 1;
> +}
> +
> +// ensure a certain boundary for the frame size
> +// frame length - 1 in ALSSpecificConfig is 16-bit, so max value is 65536
> +// frame size == 1 is not allowed because it is used in ffmpeg as a
> +// special-case value to indicate PCM audio
> +avctx->frame_size   = av_clip(avctx->frame_size, 2, 65536);
> +sconf->frame_length = avctx->frame_size;
> +
> +// determine distance between ra-frames. 0 = no ra, 1 = all ra
> +// defaults to 10s intervals for random access
> +sconf->ra_distance = avctx->gop_size;
> +/* There is an API issue where the required output audio buffer size 
> cannot
> +   be known to the user, and the default buffer size in ffmpeg.c is too
> +   small to consistently fit more than about 7 frames.  Once this issue
> +   is resolved, the maximum value can be changed from 7 to 255. */

> +sconf->ra_distance = av_clip(sconf->ra_distance, 0, 7);

av_clip_uintp2()

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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

Re: [FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check

[Michael Niedermayer]

On Tue, Oct 04, 2016 at 01:52:02PM +0200, Hendrik Leppkes wrote:
> On Tue, Oct 4, 2016 at 1:44 PM, Hendrik Leppkes  wrote:
> > On Tue, Oct 4, 2016 at 1:23 PM, Michael Niedermayer
> >  wrote:
> >> On Tue, Oct 04, 2016 at 08:41:42AM +0200, Hendrik Leppkes wrote:
> >>> On Tue, Oct 4, 2016 at 4:05 AM, Michael Niedermayer
> >>>  wrote:
> >>> > On Sat, Oct 01, 2016 at 04:15:45PM +0200, Hendrik Leppkes wrote:
> >>> >> Decoders have previously not used AVFrame.pts, and with the upcoming
> >>> >> deprecation of pkt_pts (in favor of pts), this would lead to an 
> >>> >> errorneous
> >>> >> interpration of timestamps.
> >>> >
> >>> > I probably misunderstand the commit message but
> >>> > If code is changed in a user application that cannot really lift
> >>> > some blockage from changing a lib.
> >>> > a lib can only change in an incompaible way with (deprecation and)
> >>> > major version bump.
> >>> >
> >>>
> >>> The lib never did what this code suggests it did, not that I remember
> >>> (so at least not for a long long time).
> >>
> >> release/2.0 with
> >>
> >> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> >> index 29d5492..57c8d50 100644
> >> --- a/libavcodec/utils.c
> >> +++ b/libavcodec/utils.c
> >> @@ -2008,7 +2008,7 @@ int attribute_align_arg 
> >> avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
> >>  avci->to_free.extended_data = avci->to_free.data;
> >>  memset(picture->buf, 0, sizeof(picture->buf));
> >>  }
> >> -
> >> +av_assert0(picture->pts == 0 || picture->pts == AV_NOPTS_VALUE);
> >>  avctx->frame_number++;
> >>  av_frame_set_best_effort_timestamp(picture,
> >> guess_correct_pts(avctx,
> >>
> >> causes many tests to fail, indicating that AVFrame.pts was set for
> >> several video decoders, the ffmpeg code is audio decoder specific
> >> and i failed to find a case where it was triggered, i tried IIRC 3
> >> or so checkouts from the past
> >>
> >> so AVFrame.pts was maybe never set for decoding audio but it was set
> >> for video
> >
> > Can you extend the test to add "|| picture->pts == picture->pkt_pts"?
> > Because thats what it would be set to after the merge. A quick check
> > in the 2.0 code base looks like some decoders did set that, but to the
> > exact same value as pkt_pts (which is what the merge is proposing
> > right now as well)
> >
> 
> And I found this (after 2.0):
> http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=a1c5cc429d99216406170eac7e8352860076d3e8
> 
> Which apparently set pts for mpeg4 to a number parsed from the
> bitstream, entirely uncorrelated to container or audio timestamps, so
> using them would have been rather impractical for any real use-cases.

They can be usfull, some random examples:

playing MPEG4-ES with timing stored from the bitstream (assuming there
  is no demuxer lib used that is capable to extract them)

forensics, raw video stream could have its timing
  recovered, a video with manipulated container timestamps could be
  detected.

error correction, if the container level timestamps are lost or
  corrupted the stream level ones can be used to recreate them

There may be more, these are just some of the top of my head,
not your mainstream multimedia player stuff maybe but they arent
useless

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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

Re: [FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check

[Hendrik Leppkes]

On Tue, Oct 4, 2016 at 1:55 PM, Michael Niedermayer
 wrote:
> On Tue, Oct 04, 2016 at 10:30:24AM +0200, Hendrik Leppkes wrote:
>> On Tue, Oct 4, 2016 at 8:41 AM, Hendrik Leppkes  wrote:
>> > On Tue, Oct 4, 2016 at 4:05 AM, Michael Niedermayer
>> >  wrote:
>> >> On Sat, Oct 01, 2016 at 04:15:45PM +0200, Hendrik Leppkes wrote:
>> >>> Decoders have previously not used AVFrame.pts, and with the upcoming
>> >>> deprecation of pkt_pts (in favor of pts), this would lead to an 
>> >>> errorneous
>> >>> interpration of timestamps.
>> >>
>> >> I probably misunderstand the commit message but
>> >> If code is changed in a user application that cannot really lift
>> >> some blockage from changing a lib.
>> >> a lib can only change in an incompaible way with (deprecation and)
>> >> major version bump.
>> >>
>> >
>> > The lib never did what this code suggests it did, not that I remember
>> > (so at least not for a long long time).
>> >
>>
>> Of course that could still mean that some other apps "copied" the
>> ffmpeg code and try to read this field - but is this a scenario we can
>> really control?
>>
>> The pts field in AVFrame is currently unused for decoding, nothing
>> sets it (except cuvid and openh264 or so, but those set it the same
>> way it would be set in the future, so no changes there), ffmpeg.c
>> trying to read it is a remnant from a long time ago (quick blame pins
>> it at 2012 when decoding was changed to decode_audio4).
>> I couldn't actually confirm if at that time (audio) decoders did even
>> set AVFrame.pts, considering pkt_pts already existed then.
>>
>> So is starting to set a field that was previously (at least through 2
>> or so major bumps) unused a API break?
>> Its always possible some app still tries to read it, but because its
>> never set it didn't cause any problems so far.
>>
>> The alternative is of course to keep using pkt_pts, and keep pts
>> unused for decoding, but I'm not entirely convinced there is a break
>> here.
>
> not stating any oppinion in this paragraph but
> If use of AVFrame.pts is considered a bug in the current API then past
> releases with that API need to be fixed. If they are not fixed
> testing API/ABI for 3.2 will blow up (i havnt tried it yet for 3.2
> but i did for past releases previously and mixing libs between releases
> and ffmpeg is required to not blow up, it protects against API/ABI
> breaks somewhat)
> Also release notes for 3.2 would be needed as current 3.1 would not
> mix well with a release with same soname and differently used
> AVFrame.pts. That is unless i miss something
>
> Somewhat off topic and my personal oppinion
> I think independant of field names and API, there are 3 or 4 types of
> timestamps, it would be good if user applications have some easy
> way of accessing all of them for a frame from a decoder.
> the 4 types are,
> * input AVPacket.pts based
> * input AVPacket.dts based
> * what is stored in the codec bitstream if any
> * some easy to use one that simple apps can just use and not need to
>   worry about anything, aka one that is "correct" in almost all cases
>

Well we have 3 of those, and nothing that provides the codec
timestamps (not sure they would even exist anywhere).
If the 4th kind is added, it should definitely not be in AVFrame.pts
however, since people might mistakenly use that as a general purpose
timestamp.

The entire reason for the upcoming change is simple:
- AVFrame.pts is used for the timestamp in filtering and encoding, but
unused in decoding, which is inconsistent
- AVFrame.pkt_pts is only used for decoding, unused anywhere else.

So combining those into one field seems like a logical step.
AVFrame.pts has been unused for decoding for ~3 years (including
several major bumps), and even before that its use was probably not
very useful.

Is that enough time to recycle it? Or should we rather skip the change
and possibly queue it for later (ie. the next bump)?

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

Re: [FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check

[Michael Niedermayer]

On Tue, Oct 04, 2016 at 10:30:24AM +0200, Hendrik Leppkes wrote:
> On Tue, Oct 4, 2016 at 8:41 AM, Hendrik Leppkes  wrote:
> > On Tue, Oct 4, 2016 at 4:05 AM, Michael Niedermayer
> >  wrote:
> >> On Sat, Oct 01, 2016 at 04:15:45PM +0200, Hendrik Leppkes wrote:
> >>> Decoders have previously not used AVFrame.pts, and with the upcoming
> >>> deprecation of pkt_pts (in favor of pts), this would lead to an errorneous
> >>> interpration of timestamps.
> >>
> >> I probably misunderstand the commit message but
> >> If code is changed in a user application that cannot really lift
> >> some blockage from changing a lib.
> >> a lib can only change in an incompaible way with (deprecation and)
> >> major version bump.
> >>
> >
> > The lib never did what this code suggests it did, not that I remember
> > (so at least not for a long long time).
> >
> 
> Of course that could still mean that some other apps "copied" the
> ffmpeg code and try to read this field - but is this a scenario we can
> really control?
> 
> The pts field in AVFrame is currently unused for decoding, nothing
> sets it (except cuvid and openh264 or so, but those set it the same
> way it would be set in the future, so no changes there), ffmpeg.c
> trying to read it is a remnant from a long time ago (quick blame pins
> it at 2012 when decoding was changed to decode_audio4).
> I couldn't actually confirm if at that time (audio) decoders did even
> set AVFrame.pts, considering pkt_pts already existed then.
> 
> So is starting to set a field that was previously (at least through 2
> or so major bumps) unused a API break?
> Its always possible some app still tries to read it, but because its
> never set it didn't cause any problems so far.
> 
> The alternative is of course to keep using pkt_pts, and keep pts
> unused for decoding, but I'm not entirely convinced there is a break
> here.

not stating any oppinion in this paragraph but
If use of AVFrame.pts is considered a bug in the current API then past
releases with that API need to be fixed. If they are not fixed
testing API/ABI for 3.2 will blow up (i havnt tried it yet for 3.2
but i did for past releases previously and mixing libs between releases
and ffmpeg is required to not blow up, it protects against API/ABI
breaks somewhat)
Also release notes for 3.2 would be needed as current 3.1 would not
mix well with a release with same soname and differently used
AVFrame.pts. That is unless i miss something

Somewhat off topic and my personal oppinion
I think independant of field names and API, there are 3 or 4 types of
timestamps, it would be good if user applications have some easy
way of accessing all of them for a frame from a decoder.
the 4 types are,
* input AVPacket.pts based
* input AVPacket.dts based
* what is stored in the codec bitstream if any
* some easy to use one that simple apps can just use and not need to
  worry about anything, aka one that is "correct" in almost all cases


And of course i want the next release to work for distros and not
see complaints about it causing pain due to API/ABI stuff, breaking
packages and so on.

And i want a clean, easy to use, maintainable and yet powerfull API

About the original patch in this thread itself i have no real oppinon

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

Democracy is the form of government in which you can choose your dictator


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

Re: [FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check

[Hendrik Leppkes]

On Tue, Oct 4, 2016 at 1:44 PM, Hendrik Leppkes  wrote:
> On Tue, Oct 4, 2016 at 1:23 PM, Michael Niedermayer
>  wrote:
>> On Tue, Oct 04, 2016 at 08:41:42AM +0200, Hendrik Leppkes wrote:
>>> On Tue, Oct 4, 2016 at 4:05 AM, Michael Niedermayer
>>>  wrote:
>>> > On Sat, Oct 01, 2016 at 04:15:45PM +0200, Hendrik Leppkes wrote:
>>> >> Decoders have previously not used AVFrame.pts, and with the upcoming
>>> >> deprecation of pkt_pts (in favor of pts), this would lead to an 
>>> >> errorneous
>>> >> interpration of timestamps.
>>> >
>>> > I probably misunderstand the commit message but
>>> > If code is changed in a user application that cannot really lift
>>> > some blockage from changing a lib.
>>> > a lib can only change in an incompaible way with (deprecation and)
>>> > major version bump.
>>> >
>>>
>>> The lib never did what this code suggests it did, not that I remember
>>> (so at least not for a long long time).
>>
>> release/2.0 with
>>
>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> index 29d5492..57c8d50 100644
>> --- a/libavcodec/utils.c
>> +++ b/libavcodec/utils.c
>> @@ -2008,7 +2008,7 @@ int attribute_align_arg 
>> avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
>>  avci->to_free.extended_data = avci->to_free.data;
>>  memset(picture->buf, 0, sizeof(picture->buf));
>>  }
>> -
>> +av_assert0(picture->pts == 0 || picture->pts == AV_NOPTS_VALUE);
>>  avctx->frame_number++;
>>  av_frame_set_best_effort_timestamp(picture,
>> guess_correct_pts(avctx,
>>
>> causes many tests to fail, indicating that AVFrame.pts was set for
>> several video decoders, the ffmpeg code is audio decoder specific
>> and i failed to find a case where it was triggered, i tried IIRC 3
>> or so checkouts from the past
>>
>> so AVFrame.pts was maybe never set for decoding audio but it was set
>> for video
>
> Can you extend the test to add "|| picture->pts == picture->pkt_pts"?
> Because thats what it would be set to after the merge. A quick check
> in the 2.0 code base looks like some decoders did set that, but to the
> exact same value as pkt_pts (which is what the merge is proposing
> right now as well)
>

And I found this (after 2.0):
http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=a1c5cc429d99216406170eac7e8352860076d3e8

Which apparently set pts for mpeg4 to a number parsed from the
bitstream, entirely uncorrelated to container or audio timestamps, so
using them would have been rather impractical for any real use-cases.

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

Re: [FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check

[Hendrik Leppkes]

On Tue, Oct 4, 2016 at 1:23 PM, Michael Niedermayer
 wrote:
> On Tue, Oct 04, 2016 at 08:41:42AM +0200, Hendrik Leppkes wrote:
>> On Tue, Oct 4, 2016 at 4:05 AM, Michael Niedermayer
>>  wrote:
>> > On Sat, Oct 01, 2016 at 04:15:45PM +0200, Hendrik Leppkes wrote:
>> >> Decoders have previously not used AVFrame.pts, and with the upcoming
>> >> deprecation of pkt_pts (in favor of pts), this would lead to an errorneous
>> >> interpration of timestamps.
>> >
>> > I probably misunderstand the commit message but
>> > If code is changed in a user application that cannot really lift
>> > some blockage from changing a lib.
>> > a lib can only change in an incompaible way with (deprecation and)
>> > major version bump.
>> >
>>
>> The lib never did what this code suggests it did, not that I remember
>> (so at least not for a long long time).
>
> release/2.0 with
>
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 29d5492..57c8d50 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -2008,7 +2008,7 @@ int attribute_align_arg 
> avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
>  avci->to_free.extended_data = avci->to_free.data;
>  memset(picture->buf, 0, sizeof(picture->buf));
>  }
> -
> +av_assert0(picture->pts == 0 || picture->pts == AV_NOPTS_VALUE);
>  avctx->frame_number++;
>  av_frame_set_best_effort_timestamp(picture,
> guess_correct_pts(avctx,
>
> causes many tests to fail, indicating that AVFrame.pts was set for
> several video decoders, the ffmpeg code is audio decoder specific
> and i failed to find a case where it was triggered, i tried IIRC 3
> or so checkouts from the past
>
> so AVFrame.pts was maybe never set for decoding audio but it was set
> for video

Can you extend the test to add "|| picture->pts == picture->pkt_pts"?
Because thats what it would be set to after the merge. A quick check
in the 2.0 code base looks like some decoders did set that, but to the
exact same value as pkt_pts (which is what the merge is proposing
right now as well)

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

Re: [FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check

[Michael Niedermayer]

On Tue, Oct 04, 2016 at 08:41:42AM +0200, Hendrik Leppkes wrote:
> On Tue, Oct 4, 2016 at 4:05 AM, Michael Niedermayer
>  wrote:
> > On Sat, Oct 01, 2016 at 04:15:45PM +0200, Hendrik Leppkes wrote:
> >> Decoders have previously not used AVFrame.pts, and with the upcoming
> >> deprecation of pkt_pts (in favor of pts), this would lead to an errorneous
> >> interpration of timestamps.
> >
> > I probably misunderstand the commit message but
> > If code is changed in a user application that cannot really lift
> > some blockage from changing a lib.
> > a lib can only change in an incompaible way with (deprecation and)
> > major version bump.
> >
> 
> The lib never did what this code suggests it did, not that I remember
> (so at least not for a long long time).

release/2.0 with

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 29d5492..57c8d50 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -2008,7 +2008,7 @@ int attribute_align_arg 
avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
 avci->to_free.extended_data = avci->to_free.data;
 memset(picture->buf, 0, sizeof(picture->buf));
 }
-
+av_assert0(picture->pts == 0 || picture->pts == AV_NOPTS_VALUE);
 avctx->frame_number++;
 av_frame_set_best_effort_timestamp(picture,
guess_correct_pts(avctx,

causes many tests to fail, indicating that AVFrame.pts was set for
several video decoders, the ffmpeg code is audio decoder specific
and i failed to find a case where it was triggered, i tried IIRC 3
or so checkouts from the past

so AVFrame.pts was maybe never set for decoding audio but it was set
for video

make: *** [fate-force_key_frames] Error 134
make: *** [fate-vsynth1-h263] Error 134
make: *** [fate-vsynth1-h263-obmc] Error 134
make: *** [fate-vsynth1-h263p] Error 134
make: *** [fate-vsynth1-mpeg4-rc] Error 134
make: *** [fate-vsynth1-mpeg4] Error 134
make: *** [fate-vsynth1-mpeg4-error] Error 134
make: *** [fate-vsynth1-mpeg4-nr] Error 134
make: *** [fate-vsynth1-mpeg4-adv] Error 134
make: *** [fate-vsynth1-mpeg4-thread] Error 134
make: *** [fate-vsynth1-mpeg4-qpel] Error 134
make: *** [fate-vsynth1-mpeg4-adap] Error 134
make: *** [fate-vsynth1-mpeg4-qprd] Error 134
make: *** [fate-vsynth2-h263] Error 134
make: *** [fate-vsynth2-h263-obmc] Error 134
make: *** [fate-vsynth2-h263p] Error 134
make: *** [fate-vsynth2-mpeg4] Error 134
make: *** [fate-vsynth2-mpeg4-rc] Error 134
make: *** [fate-vsynth2-mpeg4-adv] Error 134
make: *** [fate-vsynth2-mpeg4-error] Error 134
make: *** [fate-vsynth2-mpeg4-thread] Error 134
make: *** [fate-vsynth2-mpeg4-nr] Error 134
make: *** [fate-vsynth2-mpeg4-adap] Error 134
make: *** [fate-vsynth2-mpeg4-qprd] Error 134
make: *** [fate-vsynth2-mpeg4-qpel] Error 134
make: *** [fate-lavf-avi] Error 134
make: *** [fate-lavf-gif] Error 134
make: *** [fate-lavf-mkv] Error 134
make: *** [fate-lavf-ismv] Error 134
make: *** [fate-lavf-mov] Error 134
make: *** [fate-lavf-nut] Error 134
make: *** [fate-gif-color] Error 134
make: *** [fate-gif-disposal-background] Error 134
make: *** [fate-gif-disposal-restore] Error 134
make: *** [fate-gif-gray] Error 134
make: *** [fate-gifenc-rgb8] Error 134
make: *** [fate-gifenc-bgr8] Error 134
make: *** [fate-gifenc-rgb4_byte] Error 134
make: *** [fate-gifenc-gray] Error 134
make: *** [fate-gifenc-bgr4_byte] Error 134
make: *** [fate-gifenc-pal8] Error 134

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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

[FFmpeg-devel] [PATCH]configure: Enable pie for toolchain=hardened.

[Carl Eugen Hoyos]

Hi!

Sorry if I miss something but with this patch, the hardening_check 
script succeeds here both for x86_32 and x86_64 (static and shared).

Please comment, Carl Eugen
From 3c5df95a022e9148f753dd2a850570080740c602 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Tue, 4 Oct 2016 12:21:41 +0200
Subject: [PATCH] configure: Enable pie for toolchain=hardened.

---
 configure |2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure b/configure
index f593191..858f2a6 100755
--- a/configure
+++ b/configure
@@ -3577,6 +3577,8 @@ case "$toolchain" in
 add_cppflags -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2
 add_cflags   -fno-strict-overflow -fstack-protector-all
 add_ldflags  -Wl,-z,relro -Wl,-z,now
+add_cflags   -fPIE
+add_ldexeflags -fPIE -pie
 ;;
 ?*)
 die "Unknown toolchain $toolchain"
-- 
1.7.10.4

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

Re: [FFmpeg-devel] [PATCH] DXVA2: Fix crash releasing IDirect3D9Surface's with av_buffer_default_free instead of Release

[Hendrik Leppkes]

Please don't top post on the mailing list.

On Tue, Oct 4, 2016 at 8:20 AM, Min  wrote:
> Hi, I'm using ffmpeg dxva2 implementation (ffmpeg_dxva2.c) to render mp4
> video to a DX texture.
>
> The crash happens when closing my application, when I call to
> av_frame_free() to the last frame I've created to decompress video. The
> hwcontext references go to 0 and internal structs start to free its memory,
> but with DXVA2FramesContext::surfaces_internal I've noticed that is an
> array with IDirect3D9Surfaces's. You can see it in the file
> libavutil/hwcontext_dxva2.c:169, inside function dxva2_init_pool().
>
> hr = IDirectXVideoAccelerationService_CreateSurface(s->service,
> ctx->width,
> ctx->height,
>
> ctx->initial_pool_size - 1,
> s->format,
> D3DPOOL_DEFAULT, 0,
>
> frames_hwctx->surface_type,
>
> s->surfaces_internal, NULL);
>
> In the same file, the function dxva2_pool_alloc() calls av_buffer_create()
> with every surface created before, but the "free" function pointer
> parameter is passed as NULL, and in that case av_buffer_default_free() will
> be called for every surface, and that's not correct. I've created the patch
> to avoid this case, and I've checked that there is no memory leaks using
> debug CRT with visual studio. The surfaces are properly released in the
> function dxva2_frames_uninit().

The surfaces are always released, this is about memory, not surfaces.
It sounds to me like your copy of the implementation here is just
flawed, and you need to increase the refcount of the pool somewhere.

The hwcontext implementation seems fine. The free is required or it'll
leak memory.

>
> I don't know exactly how to reproduce using ffmpeg command line because
> what I'm doing is integrating ffmeg in my 3d engine, but I supose that if
> you decompress using dxva2 it will crash when finishing ffmpeg app, if all
> the reference counting is ok and all the context and frames are freed.
>

the ffmpeg app does not crash when you use dxva2 decoding.

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

Re: [FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check

[Hendrik Leppkes]

On Tue, Oct 4, 2016 at 8:41 AM, Hendrik Leppkes  wrote:
> On Tue, Oct 4, 2016 at 4:05 AM, Michael Niedermayer
>  wrote:
>> On Sat, Oct 01, 2016 at 04:15:45PM +0200, Hendrik Leppkes wrote:
>>> Decoders have previously not used AVFrame.pts, and with the upcoming
>>> deprecation of pkt_pts (in favor of pts), this would lead to an errorneous
>>> interpration of timestamps.
>>
>> I probably misunderstand the commit message but
>> If code is changed in a user application that cannot really lift
>> some blockage from changing a lib.
>> a lib can only change in an incompaible way with (deprecation and)
>> major version bump.
>>
>
> The lib never did what this code suggests it did, not that I remember
> (so at least not for a long long time).
>

Of course that could still mean that some other apps "copied" the
ffmpeg code and try to read this field - but is this a scenario we can
really control?

The pts field in AVFrame is currently unused for decoding, nothing
sets it (except cuvid and openh264 or so, but those set it the same
way it would be set in the future, so no changes there), ffmpeg.c
trying to read it is a remnant from a long time ago (quick blame pins
it at 2012 when decoding was changed to decode_audio4).
I couldn't actually confirm if at that time (audio) decoders did even
set AVFrame.pts, considering pkt_pts already existed then.

So is starting to set a field that was previously (at least through 2
or so major bumps) unused a API break?
Its always possible some app still tries to read it, but because its
never set it didn't cause any problems so far.

The alternative is of course to keep using pkt_pts, and keep pts
unused for decoding, but I'm not entirely convinced there is a break
here.

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

Re: [FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check

[Hendrik Leppkes]

On Tue, Oct 4, 2016 at 4:05 AM, Michael Niedermayer
 wrote:
> On Sat, Oct 01, 2016 at 04:15:45PM +0200, Hendrik Leppkes wrote:
>> Decoders have previously not used AVFrame.pts, and with the upcoming
>> deprecation of pkt_pts (in favor of pts), this would lead to an errorneous
>> interpration of timestamps.
>
> I probably misunderstand the commit message but
> If code is changed in a user application that cannot really lift
> some blockage from changing a lib.
> a lib can only change in an incompaible way with (deprecation and)
> major version bump.
>

The lib never did what this code suggests it did, not that I remember
(so at least not for a long long time).

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

Re: [FFmpeg-devel] [PATCH] DXVA2: Fix crash releasing IDirect3D9Surface's with av_buffer_default_free instead of Release

[Min]

Hi, I'm using ffmpeg dxva2 implementation (ffmpeg_dxva2.c) to render mp4
video to a DX texture.

The crash happens when closing my application, when I call to
av_frame_free() to the last frame I've created to decompress video. The
hwcontext references go to 0 and internal structs start to free its memory,
but with DXVA2FramesContext::surfaces_internal I've noticed that is an
array with IDirect3D9Surfaces's. You can see it in the file
libavutil/hwcontext_dxva2.c:169, inside function dxva2_init_pool().

hr = IDirectXVideoAccelerationService_CreateSurface(s->service,
ctx->width,
ctx->height,

ctx->initial_pool_size - 1,
s->format,
D3DPOOL_DEFAULT, 0,

frames_hwctx->surface_type,

s->surfaces_internal, NULL);

In the same file, the function dxva2_pool_alloc() calls av_buffer_create()
with every surface created before, but the "free" function pointer
parameter is passed as NULL, and in that case av_buffer_default_free() will
be called for every surface, and that's not correct. I've created the patch
to avoid this case, and I've checked that there is no memory leaks using
debug CRT with visual studio. The surfaces are properly released in the
function dxva2_frames_uninit().

I don't know exactly how to reproduce using ffmpeg command line because
what I'm doing is integrating ffmeg in my 3d engine, but I supose that if
you decompress using dxva2 it will crash when finishing ffmpeg app, if all
the reference counting is ok and all the context and frames are freed.

Sorry if I'm wrong, but what I've seen with the visual studio debugger and
what I can see in the code makes sense for me .

Greetings
Min

2016-09-30 8:23 GMT+02:00 Hendrik Leppkes :

> On Fri, Sep 30, 2016 at 7:48 AM, Min  wrote:
> > diff --git a/libavutil/hwcontext_dxva2.c b/libavutil/hwcontext_dxva2.c
> > index e79254b..17d8eb5 100644
> > --- a/libavutil/hwcontext_dxva2.c
> > +++ b/libavutil/hwcontext_dxva2.c
> > @@ -101,6 +101,11 @@ static void dxva2_frames_uninit(AVHWFramesContext
> *ctx)
> >  }
> >  }
> >
> > +void dxva2_pool_free(void *opaque, uint8_t *data)
> > +{
> > +// No need to free surfaces here, they will be Released later
> > +}
> > +
> >  static AVBufferRef *dxva2_pool_alloc(void *opaque, int size)
> >  {
> >  AVHWFramesContext  *ctx = (AVHWFramesContext*)opaque;
> > @@ -110,7 +115,7 @@ static AVBufferRef *dxva2_pool_alloc(void *opaque,
> int
> > size)
> >  if (s->nb_surfaces_used < hwctx->nb_surfaces) {
> >  s->nb_surfaces_used++;
> >  return
> > av_buffer_create((uint8_t*)s->surfaces_internal[s->nb_surfaces_used -
> 1],
> > -sizeof(*hwctx->surfaces), NULL, 0, 0);
> > +sizeof(*hwctx->surfaces),
> dxva2_pool_free,
> > 0, 0);
> >  }
> >
> >  return NULL;
>
> This is not correct and will leak memory. The buffer created here is
> not a "surface", its plain memory to hold information about a surface,
> and needs to be free'ed properly when its no longer used.
> On top of all that, DXVA2 usage through ffmpeg.c, for example, also
> doesn't crash here.
>
> So, how exactly does one reproduce the problem you're trying to fix?
>
> - Hendrik
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 4/4] ffprobe: report field order for video streams

[Clément Bœsch]

On Mon, Oct 03, 2016 at 11:49:39PM -0500, Rodger Combs wrote:
> ---
>  ffprobe.c   | 13 +
>  tests/ref/fate/concat-demuxer-extended-lavf-mxf |  2 +-
>  tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 |  2 +-
>  tests/ref/fate/concat-demuxer-simple1-lavf-mxf  |  2 +-
>  tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10  |  2 +-
>  tests/ref/fate/concat-demuxer-simple2-lavf-ts   |  2 +-
>  tests/ref/fate/ffprobe_compact  |  4 ++--
>  tests/ref/fate/ffprobe_csv  |  4 ++--
>  tests/ref/fate/ffprobe_default  |  2 ++
>  tests/ref/fate/ffprobe_flat |  2 ++
>  tests/ref/fate/ffprobe_ini  |  2 ++
>  11 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/ffprobe.c b/ffprobe.c
> index bb3979c..3118e80 100644
> --- a/ffprobe.c
> +++ b/ffprobe.c
> @@ -2268,6 +2268,19 @@ static int show_stream(WriterContext *w, 
> AVFormatContext *fmt_ctx, int stream_id
>  else
>  print_str_opt("chroma_location", 
> av_chroma_location_name(par->chroma_location));
>  
> +if (par->field_order == AV_FIELD_PROGRESSIVE)
> +print_str("field_order", "progressive");
> +else if (par->field_order == AV_FIELD_TT)
> +print_str("field_order", "tt");
> +else if (par->field_order == AV_FIELD_BB)
> +print_str("field_order", "bb");
> +else if (par->field_order == AV_FIELD_TB)
> +print_str("field_order", "tb");
> +else if (par->field_order == AV_FIELD_BT)
> +print_str("field_order", "bt");
> +else
> +print_str_opt("field_order", "unknown");
> +

This probably needs an update of doc/ffprobe.xsd

[...]

-- 
Clément B.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel