On Sun, 25 Oct 2015, Nicolas George wrote:

Le tridi 3 brumaire, an CCXXIV, Marton Balint a écrit :
Signed-off-by: Marton Balint <c...@passwd.hu>
---
 libavformat/concatdec.c                    | 6 ++++++
 tests/ref/fate/concat-demuxer-lavf-mxf     | 2 +-
 tests/ref/fate/concat-demuxer-lavf-mxf_d10 | 2 +-
 tests/ref/fate/concat-demuxer-lavf-ts      | 2 +-
 4 files changed, 9 insertions(+), 3 deletions(-)

I suspect that a basic run of ffmpeg to remux:

ffmpeg -i list.concat -c copy out.mkv

... would result in out.mkv having the metadata strings in it, which would
not be ok IMHO.

I couldn't actually reproduce this neither with mkv nor with nut, but I see that you are worried that the metadata may be kept in the output.

I'd rather keep this as it is, the user has to control the metadata he wants in his output, we never promised there won't be additional packet string metadata in a certain format, and the way I see it only NUT muxer actually parses AV_PKT_DATA_STRINGS_METADATA, and none of the encoders parse AVFrame metadata, so it does not really affect anybody.

If you still think this is problematic, I can do one or both of these options:
- Add a demuxer option which controls if the demuxer produces this
metadata or not.
- Add an ffconcat file global option which controls this.


diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
index f262d44..51b9703 100644
--- a/libavformat/concatdec.c
+++ b/libavformat/concatdec.c
@@ -289,6 +289,7 @@ static int open_file(AVFormatContext *avf, unsigned fileno)
 {
     ConcatContext *cat = avf->priv_data;
     ConcatFile *file = &cat->files[fileno];
+    AVDictionaryEntry *entry;
     int ret;

     if (cat->avf)
@@ -324,6 +325,11 @@ static int open_file(AVFormatContext *avf, unsigned fileno)
             file->duration -= cat->avf->duration - (file->outpoint - 
file->file_start_time);
     }


+    if (!(entry = av_dict_get(file->metadata, "lavf.concatdec.start_time", 
NULL, 0)))
+        av_dict_set_int(&file->metadata, "lavf.concatdec.start_time", 
file->start_time, 0);
+    if (!(entry = av_dict_get(file->metadata, "lavf.concatdec.duration", NULL, 
0)))
+        av_dict_set_int(&file->metadata, "lavf.concatdec.duration", 
file->duration, 0);

Since the metadata is properly namespaced, I do not think the test are
necessary.

Ok.


+
     if ((ret = match_streams(avf)) < 0)
         return ret;
     if (file->inpoint != AV_NOPTS_VALUE) {
diff --git a/tests/ref/fate/concat-demuxer-lavf-mxf 
b/tests/ref/fate/concat-demuxer-lavf-mxf
index a6fa554..d6c82d6 100644
--- a/tests/ref/fate/concat-demuxer-lavf-mxf
+++ b/tests/ref/fate/concat-demuxer-lavf-mxf
@@ -1 +1 @@

-56359998da34c3957124a8928fb58f3d 
*tests/data/fate/concat-demuxer-lavf-mxf.ffprobe
+23cd3acf3db9ee19228f381f05f1f3b9 
*tests/data/fate/concat-demuxer-lavf-mxf.ffprobe

If the ref files were not hashed, it would be easier to be sure the change
is valid.

diff --git a/tests/ref/fate/concat-demuxer-lavf-mxf_d10 
b/tests/ref/fate/concat-demuxer-lavf-mxf_d10
index 018d631..08777f7 100644
--- a/tests/ref/fate/concat-demuxer-lavf-mxf_d10
+++ b/tests/ref/fate/concat-demuxer-lavf-mxf_d10
@@ -1 +1 @@
-89c81149b4673c60aba7cf5f27cec823 
*tests/data/fate/concat-demuxer-lavf-mxf_d10.ffprobe
+bd1c6cc871fe5193186a03554ebc84c1 
*tests/data/fate/concat-demuxer-lavf-mxf_d10.ffprobe
diff --git a/tests/ref/fate/concat-demuxer-lavf-ts 
b/tests/ref/fate/concat-demuxer-lavf-ts
index 2e8ba46..a01f712 100644
--- a/tests/ref/fate/concat-demuxer-lavf-ts
+++ b/tests/ref/fate/concat-demuxer-lavf-ts
@@ -1 +1 @@
-1993b3613952fa76da8c5c260a16a96a 
*tests/data/fate/concat-demuxer-lavf-ts.ffprobe
+728e773e5009f7f652c1677573b6c8d2 
*tests/data/fate/concat-demuxer-lavf-ts.ffprobe


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

Reply via email to