Re: [FFmpeg-devel] [PATCH] mov: write colr by default
On Wed, Mar 04, 2015 at 03:40:09PM +0100, Robert Krüger wrote: On Wed, Mar 4, 2015 at 11:05 AM, Michael Niedermayer michae...@gmx.at wrote: On Wed, Mar 04, 2015 at 10:22:26AM +0100, Robert Krüger wrote: On Tue, Mar 3, 2015 at 9:27 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 09:24:17PM +0100, Robert Krüger wrote: On Tue, Mar 3, 2015 at 7:23 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 06:17:22PM +0100, Robert Krüger wrote: This is based on an earlier patch by Derek please mention this in the commit message OK, I will change that that never went in because it was argumented earlier that api breakage is not acceptable. However, that was more or less relaxed after Michael noted that the replaced flag had never been part of a release and since a number of people seem to agree, this is the better default, I am submitting this patch now, to have it in before the upcoming release. Let me know if that will be accepted and I will modify the respective fate tests as well. have you tested the generated mov and mp4 files with some common software packages ? checking random files on my disk it seems more than half the mov files contain a colr atom but i found just a single mp4 with a colr atom, so especially testing the compatibility of the mp4 files would be optimal before this is changed OK, I will do some tests with VLC, Quicktime, Final Cut, Final Cut X, Premiere and After Effects and maybe something else I find. thanks I tried an mp4 file with a colr atom with VLC, Quicktime, Final Cut X, Compressor, Premiere, After Effects and Adobe Media Encoder. None of those had any problems with the file and colors looked normal too. ok then please submit a patch that also updates fate here it is. b/libavformat/movenc.c|6 +-- b/libavformat/movenc.h|2 - b/libavformat/version.h |4 +- b/tests/fate/vcodec.mak |8 ++-- b/tests/ref/lavf/mov | 16 b/tests/ref/seek/lavf-mov | 44 +++--- b/tests/ref/vsynth/vsynth1-avui |4 +- b/tests/ref/vsynth/vsynth1-dnxhd-1080i|8 ++-- b/tests/ref/vsynth/vsynth1-dnxhd-1080i-nocolr |4 ++ b/tests/ref/vsynth/vsynth1-mpeg4 |4 +- b/tests/ref/vsynth/vsynth1-prores |4 +- b/tests/ref/vsynth/vsynth1-prores_ks |4 +- b/tests/ref/vsynth/vsynth1-qtrle |4 +- b/tests/ref/vsynth/vsynth1-qtrlegray |4 +- b/tests/ref/vsynth/vsynth1-svq1 |4 +- b/tests/ref/vsynth/vsynth2-avui |4 +- b/tests/ref/vsynth/vsynth2-dnxhd-1080i|8 ++-- b/tests/ref/vsynth/vsynth2-dnxhd-1080i-nocolr |4 ++ b/tests/ref/vsynth/vsynth2-mpeg4 |4 +- b/tests/ref/vsynth/vsynth2-prores |4 +- b/tests/ref/vsynth/vsynth2-prores_ks |4 +- b/tests/ref/vsynth/vsynth2-qtrle |4 +- b/tests/ref/vsynth/vsynth2-qtrlegray |4 +- b/tests/ref/vsynth/vsynth2-svq1 |4 +- b/tests/ref/vsynth/vsynth3-dnxhd-1080i-nocolr |4 ++ b/tests/ref/vsynth/vsynth3-mpeg4 |4 +- b/tests/ref/vsynth/vsynth3-prores |4 +- b/tests/ref/vsynth/vsynth3-prores_ks |4 +- b/tests/ref/vsynth/vsynth3-qtrle |4 +- b/tests/ref/vsynth/vsynth3-svq1 |4 +- b/tests/ref/vsynth/vsynth_lena-dnxhd-1080i-nocolr |4 ++ tests/ref/vsynth/vsynth1-dnxhd-1080i-colr |4 -- tests/ref/vsynth/vsynth2-dnxhd-1080i-colr |4 -- tests/ref/vsynth/vsynth3-dnxhd-1080i-colr |4 -- tests/ref/vsynth/vsynth_lena-dnxhd-1080i-colr |4 -- 35 files changed, 101 insertions(+), 103 deletions(-) f6cafbe678f902fd7410b7dee69d9f6e04e591c1 mov_write_colr_by_default.patch From f836ea0dcb181417b38a75302c1c5ffdeaa0fb4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Kr=C3=BCger?= krue...@lesspain.de Date: Tue, 3 Mar 2015 18:11:54 +0100 Subject: [PATCH 1/2] mov: make writing colr the default (based on a patch by derek buitenhuis) this breaks fate, the test needing the lena sample are missing an update also once one fixes this the fate scores differ between 32bit and 64bit on linux --- tests/ref/vsynth/vsynth3-dnxhd-1080i-nocolr 2015-03-04 16:03:41.759112905 +0100 +++ tests/data/fate/vsynth3-dnxhd-1080i-nocolr 2015-03-04 17:13:31.971201181
Re: [FFmpeg-devel] [PATCH] mov: write colr by default
On Wed, Mar 4, 2015 at 5:34 PM, Michael Niedermayer michae...@gmx.at wrote: On Wed, Mar 04, 2015 at 03:40:09PM +0100, Robert Krüger wrote: On Wed, Mar 4, 2015 at 11:05 AM, Michael Niedermayer michae...@gmx.at wrote: On Wed, Mar 04, 2015 at 10:22:26AM +0100, Robert Krüger wrote: On Tue, Mar 3, 2015 at 9:27 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 09:24:17PM +0100, Robert Krüger wrote: On Tue, Mar 3, 2015 at 7:23 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 06:17:22PM +0100, Robert Krüger wrote: This is based on an earlier patch by Derek please mention this in the commit message OK, I will change that that never went in because it was argumented earlier that api breakage is not acceptable. However, that was more or less relaxed after Michael noted that the replaced flag had never been part of a release and since a number of people seem to agree, this is the better default, I am submitting this patch now, to have it in before the upcoming release. Let me know if that will be accepted and I will modify the respective fate tests as well. have you tested the generated mov and mp4 files with some common software packages ? checking random files on my disk it seems more than half the mov files contain a colr atom but i found just a single mp4 with a colr atom, so especially testing the compatibility of the mp4 files would be optimal before this is changed OK, I will do some tests with VLC, Quicktime, Final Cut, Final Cut X, Premiere and After Effects and maybe something else I find. thanks I tried an mp4 file with a colr atom with VLC, Quicktime, Final Cut X, Compressor, Premiere, After Effects and Adobe Media Encoder. None of those had any problems with the file and colors looked normal too. ok then please submit a patch that also updates fate here it is. b/libavformat/movenc.c|6 +-- b/libavformat/movenc.h|2 - b/libavformat/version.h |4 +- b/tests/fate/vcodec.mak |8 ++-- b/tests/ref/lavf/mov | 16 b/tests/ref/seek/lavf-mov | 44 +++--- b/tests/ref/vsynth/vsynth1-avui |4 +- b/tests/ref/vsynth/vsynth1-dnxhd-1080i|8 ++-- b/tests/ref/vsynth/vsynth1-dnxhd-1080i-nocolr |4 ++ b/tests/ref/vsynth/vsynth1-mpeg4 |4 +- b/tests/ref/vsynth/vsynth1-prores |4 +- b/tests/ref/vsynth/vsynth1-prores_ks |4 +- b/tests/ref/vsynth/vsynth1-qtrle |4 +- b/tests/ref/vsynth/vsynth1-qtrlegray |4 +- b/tests/ref/vsynth/vsynth1-svq1 |4 +- b/tests/ref/vsynth/vsynth2-avui |4 +- b/tests/ref/vsynth/vsynth2-dnxhd-1080i|8 ++-- b/tests/ref/vsynth/vsynth2-dnxhd-1080i-nocolr |4 ++ b/tests/ref/vsynth/vsynth2-mpeg4 |4 +- b/tests/ref/vsynth/vsynth2-prores |4 +- b/tests/ref/vsynth/vsynth2-prores_ks |4 +- b/tests/ref/vsynth/vsynth2-qtrle |4 +- b/tests/ref/vsynth/vsynth2-qtrlegray |4 +- b/tests/ref/vsynth/vsynth2-svq1 |4 +- b/tests/ref/vsynth/vsynth3-dnxhd-1080i-nocolr |4 ++ b/tests/ref/vsynth/vsynth3-mpeg4 |4 +- b/tests/ref/vsynth/vsynth3-prores |4 +- b/tests/ref/vsynth/vsynth3-prores_ks |4 +- b/tests/ref/vsynth/vsynth3-qtrle |4 +- b/tests/ref/vsynth/vsynth3-svq1 |4 +- b/tests/ref/vsynth/vsynth_lena-dnxhd-1080i-nocolr |4 ++ tests/ref/vsynth/vsynth1-dnxhd-1080i-colr |4 -- tests/ref/vsynth/vsynth2-dnxhd-1080i-colr |4 -- tests/ref/vsynth/vsynth3-dnxhd-1080i-colr |4 -- tests/ref/vsynth/vsynth_lena-dnxhd-1080i-colr |4 -- 35 files changed, 101 insertions(+), 103 deletions(-) f6cafbe678f902fd7410b7dee69d9f6e04e591c1 mov_write_colr_by_default.patch From f836ea0dcb181417b38a75302c1c5ffdeaa0fb4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Kr=C3=BCger?= krue...@lesspain.de Date: Tue, 3 Mar 2015 18:11:54 +0100 Subject: [PATCH 1/2] mov: make writing colr the default (based on a patch by derek buitenhuis) this breaks fate, the test needing the lena sample are missing an update Strange, I adjusted the lena sample tests
Re: [FFmpeg-devel] [PATCH 2/2] acvodec/lipopenjpeg: Improve XYZ color space detection
On Wed, Mar 04, 2015 at 05:07:17PM +0200, Vilius Grigaliūnas wrote: On Wed, Mar 4, 2015 at 4:48 PM, Michael Niedermayer michae...@gmx.at wrote: i did not mean to suggest to leave libopenjpeg broken. It is not technically broken, it just relies on guessing in some cases which might not always be correct. It think there should be a warning that this is happening and a way to override it. yes, i agree but needing to manually override any sizeable amount of real world files is not very elegant [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Awnsering whenever a program halts or runs forever is On a turing machine, in general impossible (turings halting problem). On any real computer, always possible as a real computer has a finite number of states N, and will either halt in less than N cycles or never halt. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v3 2/2] libavformat/mxfenc: write package name metadata
On Wed, Mar 4, 2015 at 4:07 AM, tomas.har...@codemill.se wrote:
On 2015-03-03 05:06, Mark Reid wrote:
+/*
+ * Returns the calculated length a local tag containing an utf-8
string as utf-16
+ */
+static uint64_t mxf_utf16_local_tag_length(const char *utf8_str)
+{
+return utf8_str? 4 + mxf_utf16len(utf8_str) * 2: 0;
Should return zero if mxf_utf16len()*2 65535.
+}
+
+/*
+ * Write a local tag containing an utf-8 string as utf-16
*/
static void mxf_write_local_tag_utf16(AVIOContext *pb, int tag, const
char *value)
{
-int i, size = strlen(value);
+int size = mxf_utf16len(value);
mxf_write_local_tag(pb, size*2, tag);
Same here. Check that size*2 65536. I don't want us outputting illegal
MXFs.
-for (i = 0; i size; i++)
-avio_wb16(pb, value[i]);
+avio_put_str16be(pb, value);
}
I did put a size check in mxf_write_package before writing the tags here:
-static void mxf_write_package(AVFormatContext *s, enum MXFMetadataSetType
type)
+static void mxf_write_package(AVFormatContext *s, enum
MXFMetadataSetType type, const char *package_name)
{
MXFContext *mxf = s-priv_data;
AVIOContext *pb = s-pb;
int i, track_count = s-nb_streams+1;
+uint64_t name_size = 0;
+
+if (package_name)
+name_size = mxf_utf16_local_tag_length(package_name);
+
+if (name_size = INT16_MAX){
+av_log(s, AV_LOG_WARNING, package name size %PRIx64 invalid
(too large), ignoring\n, name_size);
+name_size = 0;
+}
(hmm but looks like it should be UINT16_MAX instead anyway). I'll move the
check to those functions instead and send a new patch. Thanks for taking
the time to review.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mov: write colr by default
On Wed, Mar 4, 2015 at 6:57 PM, Michael Niedermayer michae...@gmx.at wrote: On Wed, Mar 04, 2015 at 06:19:19PM +0100, Robert Krüger wrote: On Wed, Mar 4, 2015 at 5:34 PM, Michael Niedermayer michae...@gmx.at wrote: On Wed, Mar 04, 2015 at 03:40:09PM +0100, Robert Krüger wrote: On Wed, Mar 4, 2015 at 11:05 AM, Michael Niedermayer michae...@gmx.at wrote: On Wed, Mar 04, 2015 at 10:22:26AM +0100, Robert Krüger wrote: On Tue, Mar 3, 2015 at 9:27 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 09:24:17PM +0100, Robert Krüger wrote: On Tue, Mar 3, 2015 at 7:23 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 06:17:22PM +0100, Robert Krüger wrote: This is based on an earlier patch by Derek please mention this in the commit message OK, I will change that that never went in because it was argumented earlier that api breakage is not acceptable. However, that was more or less relaxed after Michael noted that the replaced flag had never been part of a release and since a number of people seem to agree, this is the better default, I am submitting this patch now, to have it in before the upcoming release. Let me know if that will be accepted and I will modify the respective fate tests as well. have you tested the generated mov and mp4 files with some common software packages ? checking random files on my disk it seems more than half the mov files contain a colr atom but i found just a single mp4 with a colr atom, so especially testing the compatibility of the mp4 files would be optimal before this is changed OK, I will do some tests with VLC, Quicktime, Final Cut, Final Cut X, Premiere and After Effects and maybe something else I find. thanks I tried an mp4 file with a colr atom with VLC, Quicktime, Final Cut X, Compressor, Premiere, After Effects and Adobe Media Encoder. None of those had any problems with the file and colors looked normal too. ok then please submit a patch that also updates fate here it is. b/libavformat/movenc.c|6 +-- b/libavformat/movenc.h|2 - b/libavformat/version.h |4 +- b/tests/fate/vcodec.mak |8 ++-- b/tests/ref/lavf/mov | 16 b/tests/ref/seek/lavf-mov | 44 +++--- b/tests/ref/vsynth/vsynth1-avui |4 +- b/tests/ref/vsynth/vsynth1-dnxhd-1080i|8 ++-- b/tests/ref/vsynth/vsynth1-dnxhd-1080i-nocolr |4 ++ b/tests/ref/vsynth/vsynth1-mpeg4 |4 +- b/tests/ref/vsynth/vsynth1-prores |4 +- b/tests/ref/vsynth/vsynth1-prores_ks |4 +- b/tests/ref/vsynth/vsynth1-qtrle |4 +- b/tests/ref/vsynth/vsynth1-qtrlegray |4 +- b/tests/ref/vsynth/vsynth1-svq1 |4 +- b/tests/ref/vsynth/vsynth2-avui |4 +- b/tests/ref/vsynth/vsynth2-dnxhd-1080i|8 ++-- b/tests/ref/vsynth/vsynth2-dnxhd-1080i-nocolr |4 ++ b/tests/ref/vsynth/vsynth2-mpeg4 |4 +- b/tests/ref/vsynth/vsynth2-prores |4 +- b/tests/ref/vsynth/vsynth2-prores_ks |4 +- b/tests/ref/vsynth/vsynth2-qtrle |4 +- b/tests/ref/vsynth/vsynth2-qtrlegray |4 +- b/tests/ref/vsynth/vsynth2-svq1 |4 +- b/tests/ref/vsynth/vsynth3-dnxhd-1080i-nocolr |4 ++ b/tests/ref/vsynth/vsynth3-mpeg4 |4 +- b/tests/ref/vsynth/vsynth3-prores |4 +- b/tests/ref/vsynth/vsynth3-prores_ks |4 +- b/tests/ref/vsynth/vsynth3-qtrle |4 +- b/tests/ref/vsynth/vsynth3-svq1 |4 +- b/tests/ref/vsynth/vsynth_lena-dnxhd-1080i-nocolr |4 ++ tests/ref/vsynth/vsynth1-dnxhd-1080i-colr |4 -- tests/ref/vsynth/vsynth2-dnxhd-1080i-colr |4 -- tests/ref/vsynth/vsynth3-dnxhd-1080i-colr |4 -- tests/ref/vsynth/vsynth_lena-dnxhd-1080i-colr |4 -- 35 files changed, 101 insertions(+), 103 deletions(-) f6cafbe678f902fd7410b7dee69d9f6e04e591c1 mov_write_colr_by_default.patch
Re: [FFmpeg-devel] [PATCH] mov: write colr by default
On Wed, Mar 04, 2015 at 06:19:19PM +0100, Robert Krüger wrote: On Wed, Mar 4, 2015 at 5:34 PM, Michael Niedermayer michae...@gmx.at wrote: On Wed, Mar 04, 2015 at 03:40:09PM +0100, Robert Krüger wrote: On Wed, Mar 4, 2015 at 11:05 AM, Michael Niedermayer michae...@gmx.at wrote: On Wed, Mar 04, 2015 at 10:22:26AM +0100, Robert Krüger wrote: On Tue, Mar 3, 2015 at 9:27 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 09:24:17PM +0100, Robert Krüger wrote: On Tue, Mar 3, 2015 at 7:23 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 06:17:22PM +0100, Robert Krüger wrote: This is based on an earlier patch by Derek please mention this in the commit message OK, I will change that that never went in because it was argumented earlier that api breakage is not acceptable. However, that was more or less relaxed after Michael noted that the replaced flag had never been part of a release and since a number of people seem to agree, this is the better default, I am submitting this patch now, to have it in before the upcoming release. Let me know if that will be accepted and I will modify the respective fate tests as well. have you tested the generated mov and mp4 files with some common software packages ? checking random files on my disk it seems more than half the mov files contain a colr atom but i found just a single mp4 with a colr atom, so especially testing the compatibility of the mp4 files would be optimal before this is changed OK, I will do some tests with VLC, Quicktime, Final Cut, Final Cut X, Premiere and After Effects and maybe something else I find. thanks I tried an mp4 file with a colr atom with VLC, Quicktime, Final Cut X, Compressor, Premiere, After Effects and Adobe Media Encoder. None of those had any problems with the file and colors looked normal too. ok then please submit a patch that also updates fate here it is. b/libavformat/movenc.c|6 +-- b/libavformat/movenc.h|2 - b/libavformat/version.h |4 +- b/tests/fate/vcodec.mak |8 ++-- b/tests/ref/lavf/mov | 16 b/tests/ref/seek/lavf-mov | 44 +++--- b/tests/ref/vsynth/vsynth1-avui |4 +- b/tests/ref/vsynth/vsynth1-dnxhd-1080i|8 ++-- b/tests/ref/vsynth/vsynth1-dnxhd-1080i-nocolr |4 ++ b/tests/ref/vsynth/vsynth1-mpeg4 |4 +- b/tests/ref/vsynth/vsynth1-prores |4 +- b/tests/ref/vsynth/vsynth1-prores_ks |4 +- b/tests/ref/vsynth/vsynth1-qtrle |4 +- b/tests/ref/vsynth/vsynth1-qtrlegray |4 +- b/tests/ref/vsynth/vsynth1-svq1 |4 +- b/tests/ref/vsynth/vsynth2-avui |4 +- b/tests/ref/vsynth/vsynth2-dnxhd-1080i|8 ++-- b/tests/ref/vsynth/vsynth2-dnxhd-1080i-nocolr |4 ++ b/tests/ref/vsynth/vsynth2-mpeg4 |4 +- b/tests/ref/vsynth/vsynth2-prores |4 +- b/tests/ref/vsynth/vsynth2-prores_ks |4 +- b/tests/ref/vsynth/vsynth2-qtrle |4 +- b/tests/ref/vsynth/vsynth2-qtrlegray |4 +- b/tests/ref/vsynth/vsynth2-svq1 |4 +- b/tests/ref/vsynth/vsynth3-dnxhd-1080i-nocolr |4 ++ b/tests/ref/vsynth/vsynth3-mpeg4 |4 +- b/tests/ref/vsynth/vsynth3-prores |4 +- b/tests/ref/vsynth/vsynth3-prores_ks |4 +- b/tests/ref/vsynth/vsynth3-qtrle |4 +- b/tests/ref/vsynth/vsynth3-svq1 |4 +- b/tests/ref/vsynth/vsynth_lena-dnxhd-1080i-nocolr |4 ++ tests/ref/vsynth/vsynth1-dnxhd-1080i-colr |4 -- tests/ref/vsynth/vsynth2-dnxhd-1080i-colr |4 -- tests/ref/vsynth/vsynth3-dnxhd-1080i-colr |4 -- tests/ref/vsynth/vsynth_lena-dnxhd-1080i-colr |4 -- 35 files changed, 101 insertions(+), 103 deletions(-) f6cafbe678f902fd7410b7dee69d9f6e04e591c1 mov_write_colr_by_default.patch From f836ea0dcb181417b38a75302c1c5ffdeaa0fb4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Kr=C3=BCger?= krue...@lesspain.de Date: Tue, 3 Mar 2015 18:11:54 +0100 Subject: [PATCH 1/2] mov: make
Re: [FFmpeg-devel] [PATCH] mov: write colr by default
On Wed, Mar 04, 2015 at 07:07:35PM +0100, Robert Krüger wrote: On Wed, Mar 4, 2015 at 6:57 PM, Michael Niedermayer michae...@gmx.at wrote: On Wed, Mar 04, 2015 at 06:19:19PM +0100, Robert Krüger wrote: On Wed, Mar 4, 2015 at 5:34 PM, Michael Niedermayer michae...@gmx.at wrote: On Wed, Mar 04, 2015 at 03:40:09PM +0100, Robert Krüger wrote: On Wed, Mar 4, 2015 at 11:05 AM, Michael Niedermayer michae...@gmx.at wrote: On Wed, Mar 04, 2015 at 10:22:26AM +0100, Robert Krüger wrote: On Tue, Mar 3, 2015 at 9:27 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 09:24:17PM +0100, Robert Krüger wrote: On Tue, Mar 3, 2015 at 7:23 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 06:17:22PM +0100, Robert Krüger wrote: This is based on an earlier patch by Derek please mention this in the commit message OK, I will change that that never went in because it was argumented earlier that api breakage is not acceptable. However, that was more or less relaxed after Michael noted that the replaced flag had never been part of a release and since a number of people seem to agree, this is the better default, I am submitting this patch now, to have it in before the upcoming release. Let me know if that will be accepted and I will modify the respective fate tests as well. have you tested the generated mov and mp4 files with some common software packages ? checking random files on my disk it seems more than half the mov files contain a colr atom but i found just a single mp4 with a colr atom, so especially testing the compatibility of the mp4 files would be optimal before this is changed OK, I will do some tests with VLC, Quicktime, Final Cut, Final Cut X, Premiere and After Effects and maybe something else I find. thanks I tried an mp4 file with a colr atom with VLC, Quicktime, Final Cut X, Compressor, Premiere, After Effects and Adobe Media Encoder. None of those had any problems with the file and colors looked normal too. ok then please submit a patch that also updates fate here it is. b/libavformat/movenc.c|6 +-- b/libavformat/movenc.h|2 - b/libavformat/version.h |4 +- b/tests/fate/vcodec.mak |8 ++-- b/tests/ref/lavf/mov | 16 b/tests/ref/seek/lavf-mov | 44 +++--- b/tests/ref/vsynth/vsynth1-avui |4 +- b/tests/ref/vsynth/vsynth1-dnxhd-1080i|8 ++-- b/tests/ref/vsynth/vsynth1-dnxhd-1080i-nocolr |4 ++ b/tests/ref/vsynth/vsynth1-mpeg4 |4 +- b/tests/ref/vsynth/vsynth1-prores |4 +- b/tests/ref/vsynth/vsynth1-prores_ks |4 +- b/tests/ref/vsynth/vsynth1-qtrle |4 +- b/tests/ref/vsynth/vsynth1-qtrlegray |4 +- b/tests/ref/vsynth/vsynth1-svq1 |4 +- b/tests/ref/vsynth/vsynth2-avui |4 +- b/tests/ref/vsynth/vsynth2-dnxhd-1080i|8 ++-- b/tests/ref/vsynth/vsynth2-dnxhd-1080i-nocolr |4 ++ b/tests/ref/vsynth/vsynth2-mpeg4 |4 +- b/tests/ref/vsynth/vsynth2-prores |4 +- b/tests/ref/vsynth/vsynth2-prores_ks |4 +- b/tests/ref/vsynth/vsynth2-qtrle |4 +- b/tests/ref/vsynth/vsynth2-qtrlegray |4 +- b/tests/ref/vsynth/vsynth2-svq1 |4 +- b/tests/ref/vsynth/vsynth3-dnxhd-1080i-nocolr |4 ++ b/tests/ref/vsynth/vsynth3-mpeg4 |4 +- b/tests/ref/vsynth/vsynth3-prores |4 +- b/tests/ref/vsynth/vsynth3-prores_ks |4 +- b/tests/ref/vsynth/vsynth3-qtrle |4 +- b/tests/ref/vsynth/vsynth3-svq1 |4 +- b/tests/ref/vsynth/vsynth_lena-dnxhd-1080i-nocolr |4 ++ tests/ref/vsynth/vsynth1-dnxhd-1080i-colr |4 -- tests/ref/vsynth/vsynth2-dnxhd-1080i-colr |4 -- tests/ref/vsynth/vsynth3-dnxhd-1080i-colr |4 --
Re: [FFmpeg-devel] Patch review: av_dict: add support for empty values
On Wed, Mar 4, 2015 at 9:42 AM, Michael Niedermayer michae...@gmx.at wrote: On Wed, Mar 04, 2015 at 02:24:58PM +0100, Michael Niedermayer wrote: On Wed, Mar 04, 2015 at 02:11:42PM +0100, Michael Niedermayer wrote: On Tue, Mar 03, 2015 at 10:51:01PM -0400, Peter Cordes wrote: [...] Anyway, the av_dict change is the one that requires the most review, so I'll make this email focus on that set of changes. https://github.com/FFmpeg/FFmpeg/pull/118 pull req #3, patch #1 review -char *ret = out, *end = out; +char *ret = out, *end_quote = out; why ? Because it took me a couple minutes to see that was all end was doing. I wasn't sure what it was the end of. (At first, I didn't even realize that this function was handling quoting as well as tokenizing.) +ret = av_realloc(ret, out - ret + 2); +// if realloc fails to shrink, we're hosed anyway; just leak the old buffer return ret; Yeah, I should have collapsed that bad idea / fix before pushing. I wanted to leave the code shorter, but then I realized I was starting to write a whole paragraph in a commit message to justify a one-line shortcut, so it was probably a bad idea. :P if realloc fails to shrink, the original unshrunk array should be returned to avoid the leak and failure ahh, i see you fix this in a later commit, this should be stashed with the patch that would add the bug if it was pushed and now i see you mentioned that in your mail, i guess replying to the first part and then reviewing some of the code before reading the rest of the mail was not really a great idea I have ADHD, I have a hard time keeping my emails under 10 paragraphs of different ideas. :P My main desktop just developed some sort of instability, maybe hardware. It might be a few days before I update these patches, if I don't get to it on my laptop. I guess ffmpeg prefers splitting changes into MANY separate commits for more accurate bisection. I knew that, but didn't realize what level of splitting up was aimed for. Will do for all my other patches, now that I have a better idea of what might actually get accepted. (esp. the libx264.c commit has at least 3 commits worth of changes.) Several of the patches on my other branches are already single-item changes that are ready to go in, though. (Other than the mpdecimate one where I thought there was a bug passing passing an invalid pointer as a log context. I'm still learning my way around ffmpeg. Thanks for the patch review on that.) Especially the vf_showinfo change is simple and useful. I also have a patch I didn't put on github; a non-working attempt to enable 10bit libx264rgb (it crashes). If anyone's already tried high-depth libx264rgb, I'm not finding it easily with google. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [libav-devel] [PATCH 2/2] lavf: move internal fields from public to internal context
Hi, On 06.02.2015 14:53, wm4 wrote: This is not an API change; the fields were explicitly declared private before. Unfortunately XBMC is using these semi-private fields, so it gets broken by this change. Therefore I think it would be better to postpone this until after a SOVERSION bump. Looking a bit closer at the source [1], it seems XBMC uses this only to provide a ff_read_frame_flush equivalent called xbmc_read_frame_flush. To avoid having to do that in XBMC I suggest to rename ff_read_frame_flush back to av_read_frame_flush and add it to the public API. Thoughts? Best regards, Andreas 1: https://anonscm.debian.org/cgit/pkg-multimedia/xbmc.git/tree/lib/xbmc-dll-symbols/DllAvFormat.c#n79 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [libav-devel] [PATCH 2/2] lavf: move internal fields from public to internal context
On Wed, 04 Mar 2015 20:21:26 +0100 Andreas Cadhalpun andreas.cadhal...@googlemail.com wrote: Hi, On 06.02.2015 14:53, wm4 wrote: This is not an API change; the fields were explicitly declared private before. Unfortunately XBMC is using these semi-private fields, so it gets broken by this change. Therefore I think it would be better to postpone this until after a SOVERSION bump. Looking a bit closer at the source [1], it seems XBMC uses this only to provide a ff_read_frame_flush equivalent called xbmc_read_frame_flush. To avoid having to do that in XBMC I suggest to rename ff_read_frame_flush back to av_read_frame_flush and add it to the public API. Thoughts? I don't think this is a valid excuse. Tell XBMC to fix their code. Or do I get to have API exceptions with my own project too? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH v5] libavformat/mxfenc: write package name metadata
changes since v4: * added UINT16_MAX check in mxf_utf16_local_tag_length and mxf_write_local_tag_utf16 Mark Reid (1): libavformat/mxfenc: write package name metadata libavformat/mxfenc.c | 97 +-- tests/ref/lavf/mxf| 6 +-- tests/ref/lavf/mxf_d10| 2 +- tests/ref/lavf/mxf_opatom | 2 +- 4 files changed, 91 insertions(+), 16 deletions(-) -- 2.2.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH v5] libavformat/mxfenc: write package name metadata
---
libavformat/mxfenc.c | 97 +--
tests/ref/lavf/mxf| 6 +--
tests/ref/lavf/mxf_d10| 2 +-
tests/ref/lavf/mxf_opatom | 2 +-
4 files changed, 91 insertions(+), 16 deletions(-)
diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 7d35af4..ce62009 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -624,14 +624,61 @@ static void mxf_write_preface(AVFormatContext *s)
}
/*
- * Write a local tag containing an ascii string as utf-16
+ * Returns the length of the UTF-16 string, in 16-bit characters, that would
result
+ * from decoding the utf-8 string.
+ */
+static uint64_t mxf_utf16len(const char *utf8_str)
+{
+const uint8_t *q = utf8_str;
+uint64_t size = 0;
+while (*q) {
+uint32_t ch;
+GET_UTF8(ch, *q++, goto invalid;)
+if (ch 0x1)
+size++;
+else
+size += 2;
+continue;
+invalid:
+av_log(NULL, AV_LOG_ERROR, Invaid UTF8 sequence in mxf_utf16len\n\n);
+}
+size += 1;
+return size;
+}
+
+/*
+ * Returns the calculated length a local tag containing an utf-8 string as
utf-16
+ */
+static int mxf_utf16_local_tag_length(const char *utf8_str)
+{
+uint64_t size;
+
+if (!utf8_str)
+return 0;
+
+size = mxf_utf16len(utf8_str);
+if (size = UINT16_MAX || size * 2 = UINT16_MAX) {
+av_log(NULL, AV_LOG_ERROR, utf16 local tag size %PRIx64 invalid
(too large), ignoring\n, size);
+return 0;
+}
+
+return 4 + size * 2;
+}
+
+/*
+ * Write a local tag containing an utf-8 string as utf-16
*/
static void mxf_write_local_tag_utf16(AVIOContext *pb, int tag, const char
*value)
{
-int i, size = strlen(value);
+uint64_t size = mxf_utf16len(value);
+
+if (size = UINT16_MAX || size * 2 = UINT16_MAX) {
+av_log(NULL, AV_LOG_ERROR, utf16 local tag size %PRIx64 invalid
(too large), ignoring\n, size);
+return;
+}
+
mxf_write_local_tag(pb, size*2, tag);
-for (i = 0; i size; i++)
-avio_wb16(pb, value[i]);
+avio_put_str16be(pb, value);
}
static void mxf_write_identification(AVFormatContext *s)
@@ -648,7 +695,9 @@ static void mxf_write_identification(AVFormatContext *s)
version = s-flags AVFMT_FLAG_BITEXACT ?
0.0.0 : AV_STRINGIFY(LIBAVFORMAT_VERSION);
-length = 84 + (strlen(company)+strlen(product)+strlen(version))*2; //
utf-16
+length = 72 + mxf_utf16_local_tag_length(company) +
+ mxf_utf16_local_tag_length(product) +
+ mxf_utf16_local_tag_length(version);
klv_encode_ber_length(pb, length);
// write uid
@@ -659,7 +708,6 @@ static void mxf_write_identification(AVFormatContext *s)
// write generation uid
mxf_write_local_tag(pb, 16, 0x3C09);
mxf_write_uuid(pb, Identification, 1);
-
mxf_write_local_tag_utf16(pb, 0x3C01, company); // Company Name
mxf_write_local_tag_utf16(pb, 0x3C02, product); // Product Name
mxf_write_local_tag_utf16(pb, 0x3C04, version); // Version String
@@ -1092,20 +1140,21 @@ static void
mxf_write_generic_sound_desc(AVFormatContext *s, AVStream *st)
mxf_write_generic_sound_common(s, st, mxf_generic_sound_descriptor_key, 0);
}
-static void mxf_write_package(AVFormatContext *s, enum MXFMetadataSetType type)
+static void mxf_write_package(AVFormatContext *s, enum MXFMetadataSetType
type, const char *package_name)
{
MXFContext *mxf = s-priv_data;
AVIOContext *pb = s-pb;
int i, track_count = s-nb_streams+1;
+int name_size = mxf_utf16_local_tag_length(package_name);
if (type == MaterialPackage) {
mxf_write_metadata_key(pb, 0x013600);
PRINT_KEY(s, Material Package key, pb-buf_ptr - 16);
-klv_encode_ber_length(pb, 92 + 16*track_count);
+klv_encode_ber_length(pb, 92 + name_size + (16*track_count));
} else {
mxf_write_metadata_key(pb, 0x013700);
PRINT_KEY(s, Source Package key, pb-buf_ptr - 16);
-klv_encode_ber_length(pb, 112 + 16*track_count); // 20 bytes length
for descriptor reference
+klv_encode_ber_length(pb, 112 + name_size + (16*track_count)); // 20
bytes length for descriptor reference
}
// write uid
@@ -1119,6 +1168,10 @@ static void mxf_write_package(AVFormatContext *s, enum
MXFMetadataSetType type)
mxf_write_umid(s, type == SourcePackage);
PRINT_KEY(s, package umid second part, pb-buf_ptr - 16);
+// package name
+if (name_size)
+mxf_write_local_tag_utf16(pb, 0x4402, package_name);
+
// package creation date
mxf_write_local_tag(pb, 8, 0x4405);
avio_wb64(pb, mxf-timestamp);
@@ -1187,11 +1240,33 @@ static int
mxf_write_essence_container_data(AVFormatContext *s)
static int mxf_write_header_metadata_sets(AVFormatContext *s)
{
+const char *material_package_name = NULL;
+const char *file_package_name = NULL;
+AVDictionaryEntry *entry = NULL;
+AVStream
Re: [FFmpeg-devel] [libav-devel] [PATCH 2/2] lavf: move internal fields from public to internal context
On Wed, Mar 04, 2015 at 09:49:17PM +0100, Andreas Cadhalpun wrote: On 04.03.2015 21:14, wm4 wrote: On Wed, 4 Mar 2015 21:05:04 +0100 Hendrik Leppkes h.lepp...@gmail.com wrote: On Wed, Mar 4, 2015 at 8:59 PM, wm4 nfx...@googlemail.com wrote: On Wed, 04 Mar 2015 20:21:26 +0100 Andreas Cadhalpun andreas.cadhal...@googlemail.com wrote: [...] Also, instead of just making some function public, maybe someone should bother to explain why we would need it? Flushing the demuxer can actually be useful. I even sent a patch once, but apparently it went nowhere. It still can be simulated with a byte seek to the current position, though. So what do you think about resurrecting this patch (add avformat_flush)? It seems to have only stalled on the associated documentation and the XBMC developers are apparently not the only ones who would find it useful. as far as iam concerned iam in favor of adding avformat_flush() can someone post a patch for that ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] GSoC and Outreachy
Students are coming on irc looking for help and guidance at all hours of the day, so if anyone feels like helping out, that would be great! Even just a few more people on irc to greet students would be nice. Some students require simple things like help with getting git installed or installing vdpau dev lib/ headers or finding trac and locating the mentors. -compn ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [FFmpeg-cvslog] vf_showinfo: minimum widths for some early fields
Le quartidi 14 ventôse, an CCXXIII, Peter Cordes a écrit : - n:%PRId64 pts:%s pts_time:%s pos:%PRId64 + n:%4PRId64 pts:%7s pts_time:%-7s pos:%9PRId64 Unless I am wrong reading the printf specifiers, this will print pts:902 instead of pts:902. Some scripts may use the output of showinfo and are not expecting the extra spaces. Sorry to not have spotted that before it was applied. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Patch review: av_dict: add support for empty values
On Wed, Mar 04, 2015 at 02:19:36PM -0400, Peter Cordes wrote: On Wed, Mar 4, 2015 at 9:42 AM, Michael Niedermayer michae...@gmx.at wrote: On Wed, Mar 04, 2015 at 02:24:58PM +0100, Michael Niedermayer wrote: On Wed, Mar 04, 2015 at 02:11:42PM +0100, Michael Niedermayer wrote: On Tue, Mar 03, 2015 at 10:51:01PM -0400, Peter Cordes wrote: [...] Anyway, the av_dict change is the one that requires the most review, so I'll make this email focus on that set of changes. https://github.com/FFmpeg/FFmpeg/pull/118 pull req #3, patch #1 review -char *ret = out, *end = out; +char *ret = out, *end_quote = out; why ? Because it took me a couple minutes to see that was all end was doing. I wasn't sure what it was the end of. (At first, I didn't even realize that this function was handling quoting as well as tokenizing.) +ret = av_realloc(ret, out - ret + 2); +// if realloc fails to shrink, we're hosed anyway; just leak the old buffer return ret; Yeah, I should have collapsed that bad idea / fix before pushing. I wanted to leave the code shorter, but then I realized I was starting to write a whole paragraph in a commit message to justify a one-line shortcut, so it was probably a bad idea. :P if realloc fails to shrink, the original unshrunk array should be returned to avoid the leak and failure ahh, i see you fix this in a later commit, this should be stashed with the patch that would add the bug if it was pushed and now i see you mentioned that in your mail, i guess replying to the first part and then reviewing some of the code before reading the rest of the mail was not really a great idea I have ADHD, I have a hard time keeping my emails under 10 paragraphs of different ideas. :P My main desktop just developed some sort of instability, maybe hardware. It might be a few days before I update these patches, if I don't get to it on my laptop. I guess ffmpeg prefers splitting changes into MANY separate commits for more accurate bisection. I knew that, but didn't realize what level of splitting up was aimed for. Will do for all my other patches, now that I have a better idea of what might actually get accepted. (esp. the libx264.c commit has at least 3 commits worth of changes.) Several of the patches on my other branches are already single-item changes that are ready to go in, though. (Other than the mpdecimate one where I thought there was a bug passing passing an invalid pointer as a log context. I'm still learning my way around ffmpeg. Thanks for the patch review on that.) Especially the vf_showinfo change is simple and useful. applied this and 3 other simple patches thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 1 Used only once- Some unspecified defect prevented a second use In good condition - Can be repaird by experienced expert As is - You wouldnt want it even if you were payed for it, if you knew ... signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [FFmpeg-cvslog] vf_showinfo: minimum widths for some early fields
On Wed, Mar 04, 2015 at 09:59:35PM +0100, Nicolas George wrote: Le quartidi 14 ventôse, an CCXXIII, Peter Cordes a écrit : - n:%PRId64 pts:%s pts_time:%s pos:%PRId64 + n:%4PRId64 pts:%7s pts_time:%-7s pos:%9PRId64 Unless I am wrong reading the printf specifiers, this will print pts:902 instead of pts:902. Some scripts may use the output of showinfo and are not expecting the extra spaces. Sorry to not have spotted that before it was applied. hmm what do you suggest we do ? revert it ? is this really a problem ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The misfortune of the wise is better than the prosperity of the fool. -- Epicurus signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/dashenc: Update extradata for mov muxer
---
libavformat/dashenc.c | 43 ---
1 file changed, 28 insertions(+), 15 deletions(-)
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 25c699e..92b7d6c 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -72,7 +72,6 @@ typedef struct OutputStream {
int bit_rate;
char bandwidth_str[64];
-int codec_str_extradata_size;
char codec_str[100];
} OutputStream;
@@ -503,12 +502,6 @@ static int write_manifest(AVFormatContext *s, int final)
if (st-codec-codec_type != AVMEDIA_TYPE_VIDEO)
continue;
-if (os-codec_str_extradata_size != st-codec-extradata_size) {
-memset(os-codec_str, 0, sizeof(os-codec_str));
-set_codec_str(s, st-codec, os-codec_str,
sizeof(os-codec_str));
-os-codec_str_extradata_size = st-codec-extradata_size;
-}
-
avio_printf(out, \t\t\tRepresentation id=\%d\
mimeType=\video/mp4\ codecs=\%s\%s width=\%d\ height=\%d\\n, i,
os-codec_str, os-bandwidth_str, st-codec-width, st-codec-height);
output_segment_list(c-streams[i], out, c);
avio_printf(out, \t\t\t/Representation\n);
@@ -524,12 +517,6 @@ static int write_manifest(AVFormatContext *s, int final)
if (st-codec-codec_type != AVMEDIA_TYPE_AUDIO)
continue;
-if (os-codec_str_extradata_size != st-codec-extradata_size) {
-memset(os-codec_str, 0, sizeof(os-codec_str));
-set_codec_str(s, st-codec, os-codec_str,
sizeof(os-codec_str));
-os-codec_str_extradata_size = st-codec-extradata_size;
-}
-
avio_printf(out, \t\t\tRepresentation id=\%d\
mimeType=\audio/mp4\ codecs=\%s\%s audioSamplingRate=\%d\\n, i,
os-codec_str, os-bandwidth_str, st-codec-sample_rate);
avio_printf(out, \t\t\t\tAudioChannelConfiguration
schemeIdUri=\urn:mpeg:dash:23003:3:audio_channel_configuration:2011\
value=\%d\ /\n, st-codec-channels);
output_segment_list(c-streams[i], out, c);
@@ -664,8 +651,7 @@ static int dash_write_header(AVFormatContext *s)
else if (st-codec-codec_type == AVMEDIA_TYPE_AUDIO)
c-has_audio = 1;
-set_codec_str(s, s-streams[i]-codec, os-codec_str,
sizeof(os-codec_str));
-os-codec_str_extradata_size = s-streams[i]-codec-extradata_size;
+set_codec_str(s, st-codec, os-codec_str, sizeof(os-codec_str));
os-first_pts = AV_NOPTS_VALUE;
os-max_pts = AV_NOPTS_VALUE;
os-segment_index = 1;
@@ -750,6 +736,29 @@ static void find_index_range(AVFormatContext *s, const
char *full_path,
*index_length = AV_RB32(buf[0]);
}
+static int update_stream_extradata(AVFormatContext *s, OutputStream *os,
+ AVCodecContext *codec)
+{
+uint8_t *extradata;
+
+if (os-ctx-streams[0]-codec-extradata_size || !codec-extradata_size)
+return 0;
+
+extradata = av_malloc(codec-extradata_size);
+
+if (!extradata)
+return AVERROR(ENOMEM);
+
+memcpy(extradata, codec-extradata, codec-extradata_size);
+
+os-ctx-streams[0]-codec-extradata = extradata;
+os-ctx-streams[0]-codec-extradata_size = codec-extradata_size;
+
+set_codec_str(s, codec, os-codec_str, sizeof(os-codec_str));
+
+return 0;
+}
+
static int dash_flush(AVFormatContext *s, int final, int stream)
{
DASHContext *c = s-priv_data;
@@ -853,6 +862,10 @@ static int dash_write_packet(AVFormatContext *s, AVPacket
*pkt)
int64_t seg_end_duration = (os-segment_index) * (int64_t)
c-min_seg_duration;
int ret;
+ret = update_stream_extradata(s, os, st-codec);
+if (ret 0)
+return ret;
+
// If forcing the stream to start at 0, the mp4 muxer will set the start
// timestamps to 0. Do the same here, to avoid mismatches in
duration/timestamps.
if (os-first_pts == AV_NOPTS_VALUE
--
2.3.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [libav-devel] [PATCH 2/2] lavf: move internal fields from public to internal context
On 3/4/2015 7:21 PM, Andreas Cadhalpun wrote: Unfortunately XBMC is using these semi-private fields, so it gets broken by this change. Therefore I think it would be better to postpone this until after a SOVERSION bump. Why exactly should we care about a project that uses explicitly private fields, which have always been documented as such? The onus is on XBMC. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [libav-devel] [PATCH 2/2] lavf: move internal fields from public to internal context
On 04.03.2015 21:14, wm4 wrote: On Wed, 4 Mar 2015 21:05:04 +0100 Hendrik Leppkes h.lepp...@gmail.com wrote: On Wed, Mar 4, 2015 at 8:59 PM, wm4 nfx...@googlemail.com wrote: On Wed, 04 Mar 2015 20:21:26 +0100 Andreas Cadhalpun andreas.cadhal...@googlemail.com wrote: Unfortunately XBMC is using these semi-private fields, so it gets broken by this change. Therefore I think it would be better to postpone this until after a SOVERSION bump. Looking a bit closer at the source [1], it seems XBMC uses this only to provide a ff_read_frame_flush equivalent called xbmc_read_frame_flush. To avoid having to do that in XBMC I suggest to rename ff_read_frame_flush back to av_read_frame_flush and add it to the public API. Thoughts? I don't think this is a valid excuse. Tell XBMC to fix their code. It's no excuse, but a problem that has to be solved one way or another. And making ff_read_frame_flush public is the easiest fix, isn't it? I agree. API explicitly marked as private needs to be possible to be changed at any time, or our development is severly impaired. That's true, but it's not entirely XBMC's fault, since the function was called av_read_frame_flush first (as if it was public) and only later renamed. (They shouldn't have worked around that by using other private functions, though.) This may be in conflict with the FFmpeg philosophy /sarcasm. Doesn't XBMC use a private copy of ffmepg anyway? They can adapt by the time they update it. Yep. At least in Debian it luckily doesn't use that, but instead the shared libraries. Also, instead of just making some function public, maybe someone should bother to explain why we would need it? Flushing the demuxer can actually be useful. I even sent a patch once, but apparently it went nowhere. It still can be simulated with a byte seek to the current position, though. So what do you think about resurrecting this patch (add avformat_flush)? It seems to have only stalled on the associated documentation and the XBMC developers are apparently not the only ones who would find it useful. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [libav-devel] [PATCH 2/2] lavf: move internal fields from public to internal context
On Wed, Mar 4, 2015 at 8:59 PM, wm4 nfx...@googlemail.com wrote: On Wed, 04 Mar 2015 20:21:26 +0100 Andreas Cadhalpun andreas.cadhal...@googlemail.com wrote: Hi, On 06.02.2015 14:53, wm4 wrote: This is not an API change; the fields were explicitly declared private before. Unfortunately XBMC is using these semi-private fields, so it gets broken by this change. Therefore I think it would be better to postpone this until after a SOVERSION bump. Looking a bit closer at the source [1], it seems XBMC uses this only to provide a ff_read_frame_flush equivalent called xbmc_read_frame_flush. To avoid having to do that in XBMC I suggest to rename ff_read_frame_flush back to av_read_frame_flush and add it to the public API. Thoughts? I don't think this is a valid excuse. Tell XBMC to fix their code. I agree. API explicitly marked as private needs to be possible to be changed at any time, or our development is severly impaired. Doesn't XBMC use a private copy of ffmepg anyway? They can adapt by the time they update it. Also, instead of just making some function public, maybe someone should bother to explain why we would need it? - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [libav-devel] [PATCH 2/2] lavf: move internal fields from public to internal context
On Wed, 4 Mar 2015 21:05:04 +0100 Hendrik Leppkes h.lepp...@gmail.com wrote: On Wed, Mar 4, 2015 at 8:59 PM, wm4 nfx...@googlemail.com wrote: On Wed, 04 Mar 2015 20:21:26 +0100 Andreas Cadhalpun andreas.cadhal...@googlemail.com wrote: Hi, On 06.02.2015 14:53, wm4 wrote: This is not an API change; the fields were explicitly declared private before. Unfortunately XBMC is using these semi-private fields, so it gets broken by this change. Therefore I think it would be better to postpone this until after a SOVERSION bump. Looking a bit closer at the source [1], it seems XBMC uses this only to provide a ff_read_frame_flush equivalent called xbmc_read_frame_flush. To avoid having to do that in XBMC I suggest to rename ff_read_frame_flush back to av_read_frame_flush and add it to the public API. Thoughts? I don't think this is a valid excuse. Tell XBMC to fix their code. I agree. API explicitly marked as private needs to be possible to be changed at any time, or our development is severly impaired. This may be in conflict with the FFmpeg philosophy /sarcasm. Doesn't XBMC use a private copy of ffmepg anyway? They can adapt by the time they update it. Yep. Also, instead of just making some function public, maybe someone should bother to explain why we would need it? Flushing the demuxer can actually be useful. I even sent a patch once, but apparently it went nowhere. It still can be simulated with a byte seek to the current position, though. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH]Support decoding prores in mxf
Hi!
Attached patch fixes ticket #4349 for me.
Please comment, Carl Eugen
diff --git a/libavformat/mxf.c b/libavformat/mxf.c
index 14d143e..61b37f8 100644
--- a/libavformat/mxf.c
+++ b/libavformat/mxf.c
@@ -50,6 +50,7 @@ const MXFCodecUL ff_mxf_codec_uls[] = {
{ {
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x01,0x32,0x00,0x00
}, 14, AV_CODEC_ID_H264 }, /* H.264/MPEG-4 AVC Intra */
{ {
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x01,0x31,0x11,0x01
}, 14, AV_CODEC_ID_H264 }, /* H.264/MPEG-4 AVC SPS/PPS in-band */
{ {
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x01,0x01,0x02,0x02,0x01
}, 16, AV_CODEC_ID_V210 }, /* V210 */
+{ {
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0E,0x04,0x02,0x01,0x02,0x11,0x04,0x00
}, 15, AV_CODEC_ID_PRORES }, /* PRORES */
/* SoundEssenceCompression */
{ {
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x03,0x04,0x02,0x02,0x02,0x03,0x03,0x01,0x00
}, 14,AV_CODEC_ID_AAC }, /* MPEG2 AAC ADTS (legacy) */
{ {
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x01,0x00,0x00,0x00,0x00
}, 13, AV_CODEC_ID_PCM_S16LE }, /* Uncompressed */
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [libav-devel] [PATCH 2/2] lavf: move internal fields from public to internal context
On Wed, 4 Mar 2015 22:33:57 +0100 Michael Niedermayer michae...@gmx.at wrote: On Wed, Mar 04, 2015 at 09:49:17PM +0100, Andreas Cadhalpun wrote: On 04.03.2015 21:14, wm4 wrote: On Wed, 4 Mar 2015 21:05:04 +0100 Hendrik Leppkes h.lepp...@gmail.com wrote: On Wed, Mar 4, 2015 at 8:59 PM, wm4 nfx...@googlemail.com wrote: On Wed, 04 Mar 2015 20:21:26 +0100 Andreas Cadhalpun andreas.cadhal...@googlemail.com wrote: [...] Also, instead of just making some function public, maybe someone should bother to explain why we would need it? Flushing the demuxer can actually be useful. I even sent a patch once, but apparently it went nowhere. It still can be simulated with a byte seek to the current position, though. So what do you think about resurrecting this patch (add avformat_flush)? It seems to have only stalled on the associated documentation and the XBMC developers are apparently not the only ones who would find it useful. as far as iam concerned iam in favor of adding avformat_flush() can someone post a patch for that ? I posted one once. You can probably find it. Though I'm not sure if XBMC can use that, or if they even care. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]Support decoding prores in mxf
On 3/4/15, Carl Eugen Hoyos ceho...@ag.or.at wrote: Hi! Attached patch fixes ticket #4349 for me. Please comment, Carl Eugen probably ok. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [libav-devel] [PATCH 2/2] lavf: move internal fields from public to internal context
On Wed, 04 Mar 2015 20:35:51 + Derek Buitenhuis derek.buitenh...@gmail.com wrote: On 3/4/2015 7:21 PM, Andreas Cadhalpun wrote: Unfortunately XBMC is using these semi-private fields, so it gets broken by this change. Therefore I think it would be better to postpone this until after a SOVERSION bump. Why exactly should we care about a project that uses explicitly private fields, which have always been documented as such? The onus is on XBMC. An extremely verbose warning too: http://0x0.st/M ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] avformat: add avformat_flush()
On Wed, Oct 01, 2014 at 07:59:12AM +0200, Reimar Döffinger wrote: On 30.09.2014, at 22:25, wm4 nfx...@googlemail.com wrote: On Tue, 30 Sep 2014 08:06:16 +0200 Reimar Döffinger reimar.doeffin...@gmx.de wrote: On 29.09.2014, at 22:02, Michael Niedermayer michae...@gmx.at wrote: On Mon, Sep 29, 2014 at 08:34:44PM +0200, wm4 wrote: On Mon, 29 Sep 2014 20:25:47 +0200 Michael Niedermayer michae...@gmx.at wrote: On Mon, Sep 29, 2014 at 07:41:28PM +0200, wm4 wrote: Useful for Bluray and DVD, since the libraries used to read them just change the byte stream under your feet on seeking. Maybe there should be a AVInputFormat callback for this. But the mpeg-ps (DVD) and the mpeg-ts (Bluray) demuxers don't change much state during seeking - they just try to find a new packet with timestamps (in read_timestamp), so I haven't found a need for this yet. I don't want to add unused things. I've also thought about adding a flush callback, and implementing them in mpeg.c and mpegts.c by just calling ff_read_frame_flush(). Might be slightly better, because you can have avformat_flush() fail on formats which don't support this? Or maybe a flag? TODO: add entry to APIchanges, bump minor version. --- libavformat/avformat.h | 13 + libavformat/utils.c| 6 ++ 2 files changed, 19 insertions(+) diff --git a/libavformat/avformat.h b/libavformat/avformat.h index 78054de..eaa52fa 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -2173,6 +2173,19 @@ int av_seek_frame(AVFormatContext *s, int stream_index, int64_t timestamp, int avformat_seek_file(AVFormatContext *s, int stream_index, int64_t min_ts, int64_t ts, int64_t max_ts, int flags); /** + * Discard all internally buffered data. This can be useful when dealing with + * discontinuities in the byte stream. Generally works only with some simple + * formats. id call them stream based or without a central header instead of simple. I can change that and replace simple with headerless. please do, headerless is more specific Why does it require headerless? I would have expected this feature to work for e.g. Ogg as well, which clearly is not headerless. As such I'd claim headerless may be more specific, but it is also wrong. It should work for all formats that can be read without index and can resync reliably at least. Do you have any concrete suggestions? I'm not sure what documentation would be most appropriate. Maybe formats that can resync. This includes headerless formats like MPEG-TS/TS but should also work with NUT, Ogg and in a limited way AVI for example Not sure that's the best way to do it either though. Changed the text to this Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When you are offended at any man's fault, turn to yourself and study your own failings. Then you will forget your anger. -- Epictetus signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [FFmpeg-cvslog] vf_showinfo: minimum widths for some early fields
On Wed, Mar 04, 2015 at 10:18:07PM +0100, Michael Niedermayer wrote: On Wed, Mar 04, 2015 at 09:59:35PM +0100, Nicolas George wrote: Le quartidi 14 ventôse, an CCXXIII, Peter Cordes a écrit : - n:%PRId64 pts:%s pts_time:%s pos:%PRId64 + n:%4PRId64 pts:%7s pts_time:%-7s pos:%9PRId64 Unless I am wrong reading the printf specifiers, this will print pts:902 instead of pts:902. Some scripts may use the output of showinfo and are not expecting the extra spaces. Sorry to not have spotted that before it was applied. hmm what do you suggest we do ? revert it ? is this really a problem ? Users should use ffprobe for a stable output. showinfo is more a debug tool than anything else, its output is not stable and never was. [...] -- Clément B. pgp_0VEjVFH5d.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/mxfdec: Detect XYZ pixel format for digital cinema files
While the native jpeg2000 decoder can determine pixel format correctly
from the codestream, libopenjpeg wrapper cannot. To make sure that
the output is correct when using libopenjpeg to decode digital
cinema files, we do detection from the metadata included in the MXF
wrapper.
If the container has JPEG 2000 Coding Parameters metadata element
with Rsiz value set to one of digital cinema profiles, we can safely
assume that the given input file is DCI compliant, therefore the
pixel format should be XYZ.
---
libavformat/mxfdec.c |7 +++
1 file changed, 7 insertions(+)
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index f3501da..2e8dd05 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -281,6 +281,7 @@ static const uint8_t mxf_encrypted_essence_container[]
= { 0x06,0x0e,0x2b,0x
static const uint8_t mxf_random_index_pack_key[] = {
0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x11,0x01,0x00
};
static const uint8_t mxf_sony_mpeg4_extradata[]= {
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0e,0x06,0x06,0x02,0x02,0x01,0x00,0x00
};
static const uint8_t mxf_avid_project_name[] = {
0xa5,0xfb,0x7b,0x25,0xf6,0x15,0x94,0xb9,0x62,0xfc,0x37,0x17,0x49,0x2d,0x42,0xbf
};
+static const uint8_t mxf_jp2k_rsiz[] = {
0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x01,0x00
};
#define IS_KLV_KEY(x, y) (!memcmp(x, y, sizeof(y)))
@@ -1000,6 +1001,12 @@ static int mxf_read_generic_descriptor(void *arg,
AVIOContext *pb, int tag, int
descriptor-extradata_size = size;
avio_read(pb, descriptor-extradata, size);
}
+if (IS_KLV_KEY(uid, mxf_jp2k_rsiz)) {
+uint32_t rsiz = avio_rb16(pb);
+if (rsiz == FF_PROFILE_JPEG2000_DCINEMA_2K ||
+rsiz == FF_PROFILE_JPEG2000_DCINEMA_4K)
+descriptor-pix_fmt = AV_PIX_FMT_XYZ12;
+}
break;
}
return 0;
--
1.7.9.5
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] configure: Check linking against CoreGraphics or, ApplicationServices framework for avfoundation input device.
Thilo Borgmann thilo.borgmann at mail.de writes: New, simpler patch attached. Please apply but check for CoreGraphics first, please. I applied this patch. Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] is_compiled flag not being cleared in av_opencl_uninit
When OpenCL kernels are compiled, is_compiled flag is being set for each
kernel. But, in opencl uninit, this flag is not being cleared.
This causes an error when an OpenCL kernel is tried on different OpenCL
devices on same platform.
Here is the patch with a fix
---
libavutil/opencl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libavutil/opencl.c b/libavutil/opencl.c
index 36cb6fe..a56029c 100644
--- a/libavutil/opencl.c
+++ b/libavutil/opencl.c
@@ -611,6 +611,9 @@ void av_opencl_uninit(void)
}
opencl_ctx.context = NULL;
}
+ for (i = 0; i opencl_ctx.kernel_code_count; i++) {
+opencl_ctx.kernel_code[i].is_compiled = 0;
+}
free_device_list(opencl_ctx.device_list);
end:
if (opencl_ctx.init_count = 0)
Please incorporate this change.
Let me know if more info is needed regarding this.
Thanks,
Srikanth
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavfi: Add support to process_command in vf_eq.c
On date Friday 2015-02-20 01:11:40 +0100, Stefano Sabatini encoded:
On date Thursday 2015-02-19 17:13:15 +0530, Arwa Arif encoded:
Updated the patch.
From 66a8c9d03995c9e7c6ccc05fb9b20756f51c17f4 Mon Sep 17 00:00:00 2001
From: Arwa Arif arwaarif1...@gmail.com
Date: Thu, 19 Feb 2015 01:26:44 +0530
Subject: [PATCH] Add process_command to eq.
---
doc/filters.texi| 35 +++
libavfilter/vf_eq.c | 171
+--
libavfilter/vf_eq.h | 56 +++--
3 files changed, 210 insertions(+), 52 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index 191b52f..e5bf3a2 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -4402,6 +4402,41 @@ Default is @code{1.0}.
@end table
+@subsection Commands
+The filter supports the following commands:
+
+@table @option
+@item contrast
+Set the contrast expression.
+
+@item brightness
+Set the brightness expression.
+
+@item saturation
+Set the saturation expression.
+
+@item gamma
+Set the gamma expression.
+
+@item gamma_r
+Set the gamma_r expression.
+
+@item gamma_g
+Set gamma_g expression.
+
+@item gamma_b
+Set gamma_b expression.
+
+@item gamma_weight
+Set gamma_weight expression.
+
+The command accepts the same syntax of the corresponding option.
What parameters do the expressions accept? Can you suggest some useful
use-case? (And add useful examples in the docs?)
+
+If the specified expression is not valid, it is kept at its current
+value.
Also, you should update the options section to make it clear that the
filter options accept expressions as well.
@arwa, can you comment about these questions?
[...]
--
FFmpeg = Formidable Fundamentalist Merciful Perfectionist Exuberant Generator
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [libav-devel] [PATCH 2/2] lavf: move internal fields from public to internal context
On 04.03.2015 23:13, Luca Barbato wrote: On 04/03/15 21:49, Andreas Cadhalpun wrote: So what do you think about resurrecting this patch (add avformat_flush)? I see no patch about it, my email client chewed it? It has been a while ago [1]. It just adds a public API for ff_read_frame_flush. It seems to have only stalled on the associated documentation and the XBMC developers are apparently not the only ones who would find it useful. The function is useful if somebody wants to implement some custom seek and in order to do that probably is using some of the fields being hidden so I'm not sure just exposing that function would do anything good for that use case. I'd rather check with the XBMC (I guess now the name is Kodi, let's try to move to use the new name) people why they need it and if the problem they have with the seeking functions can't be solved otherwise. To me it seems that this is the only internal API they need, but asking can't hurt. So CC'ing the Debian XBMC/Kodi maintainer. Best regards, Andreas 1: https://ffmpeg.org/pipermail/ffmpeg-devel/2014-September/163635.html ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]Support decoding prores in mxf
Paul B Mahol onemda at gmail.com writes: Attached patch fixes ticket #4349 for me. Please comment, Carl Eugen probably ok. Patch applied. Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] acvodec/lipopenjpeg: Improve XYZ color space detection
On Wed, Mar 4, 2015 at 1:03 AM, Michael Niedermayer michae...@gmx.at wrote: this detects codestreams_profile0/p0_08.j2k as xyz, is this intended? No, this seems to be a regression. At the moment there is not enough information in openjpeg data structures (opj_image_t specifically) to reliably determine whether given image is is 12-bit XYZ or 12-bit RGB. I do not know how the best way to handle this case. Unless we can find some additional way to determine this, one of these cases will be broken. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 07/12] mips/aacdec: remove uses of mips32r2 specific ext instructions
Removing these removes the dependency of this code on mips32r2 which would allow it to be used on processors which have FPU instructions, but not r2 instructions (like the mips64el debian port for instance). I would be more comfortable if there were two instances of this code: one for mips32r2 and one for mips32 so advantages of using mips32r2 instructions (however small here) are left intact. On the other hand, since this doesn't change much number of instructions used (adding at maximum around 100 instructions overall if I am not mistaking) I am ok with this. Well I can't see how 'ext' can ever be faster than 'and' (it does more work) so most of these should be no slower anyway. For VMUL4S my version has 2 extra instructions in it so it could be a bit slower. Does this #if seem ok? I never said that 'ext' is faster than 'and'. This code was written so that the pipeline stalls are eliminated (or reduced to a minimum). Taking this in consideration it is not important which instruction is faster overall. Maybe I was not clear, but I said that I am ok with your first patch. It is true that you only have 2 extra instructions in VMUL4S and I don't have a problem with that. Regarding your question about #if, this is even better than original patch from my point of view, but you should use HAVE_MIPS32R2 in this case. - Nedeljko ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mov: write colr by default
On Tue, Mar 3, 2015 at 9:27 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 09:24:17PM +0100, Robert Krüger wrote: On Tue, Mar 3, 2015 at 7:23 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 06:17:22PM +0100, Robert Krüger wrote: This is based on an earlier patch by Derek please mention this in the commit message OK, I will change that that never went in because it was argumented earlier that api breakage is not acceptable. However, that was more or less relaxed after Michael noted that the replaced flag had never been part of a release and since a number of people seem to agree, this is the better default, I am submitting this patch now, to have it in before the upcoming release. Let me know if that will be accepted and I will modify the respective fate tests as well. have you tested the generated mov and mp4 files with some common software packages ? checking random files on my disk it seems more than half the mov files contain a colr atom but i found just a single mp4 with a colr atom, so especially testing the compatibility of the mp4 files would be optimal before this is changed OK, I will do some tests with VLC, Quicktime, Final Cut, Final Cut X, Premiere and After Effects and maybe something else I find. thanks I tried an mp4 file with a colr atom with VLC, Quicktime, Final Cut X, Compressor, Premiere, After Effects and Adobe Media Encoder. None of those had any problems with the file and colors looked normal too. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] acvodec/lipopenjpeg: Improve XYZ color space detection
On Wed, Mar 04, 2015 at 10:08:56AM +0200, Vilius Grigaliūnas wrote: On Wed, Mar 4, 2015 at 1:03 AM, Michael Niedermayer michae...@gmx.at wrote: this detects codestreams_profile0/p0_08.j2k as xyz, is this intended? No, this seems to be a regression. At the moment there is not enough information in openjpeg data structures (opj_image_t specifically) to reliably determine whether given image is is 12-bit XYZ or 12-bit RGB. I do not know how the best way to handle this case. Unless we can find some additional way to determine this, one of these cases will be broken. the best way would be to improve our jpeg2000 decoder so its at least as good as libopenjpeg then it can be used and it would have access to everything from the bitstream to determine the colorspace ... [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The greatest way to live with honor in this world is to be what we pretend to be. -- Socrates signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mov: write colr by default
On Wed, Mar 04, 2015 at 10:22:26AM +0100, Robert Krüger wrote: On Tue, Mar 3, 2015 at 9:27 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 09:24:17PM +0100, Robert Krüger wrote: On Tue, Mar 3, 2015 at 7:23 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 06:17:22PM +0100, Robert Krüger wrote: This is based on an earlier patch by Derek please mention this in the commit message OK, I will change that that never went in because it was argumented earlier that api breakage is not acceptable. However, that was more or less relaxed after Michael noted that the replaced flag had never been part of a release and since a number of people seem to agree, this is the better default, I am submitting this patch now, to have it in before the upcoming release. Let me know if that will be accepted and I will modify the respective fate tests as well. have you tested the generated mov and mp4 files with some common software packages ? checking random files on my disk it seems more than half the mov files contain a colr atom but i found just a single mp4 with a colr atom, so especially testing the compatibility of the mp4 files would be optimal before this is changed OK, I will do some tests with VLC, Quicktime, Final Cut, Final Cut X, Premiere and After Effects and maybe something else I find. thanks I tried an mp4 file with a colr atom with VLC, Quicktime, Final Cut X, Compressor, Premiere, After Effects and Adobe Media Encoder. None of those had any problems with the file and colors looked normal too. ok then please submit a patch that also updates fate [... -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many things microsoft did are stupid, but not doing something just because microsoft did it is even more stupid. If everything ms did were stupid they would be bankrupt already. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 09/12] mips: port optimizations to mips n64
On Wed, Mar 04, 2015 at 10:10:15AM +, Nedeljko Babic wrote: LGTM seems this does not apply cleanly on HEAD Applying: mips: port optimizations to mips n64 error: patch failed: libavcodec/mips/acelp_filters_mips.c:82 error: libavcodec/mips/acelp_filters_mips.c: patch does not apply error: patch failed: libavcodec/mips/fmtconvert_mips.c:50 error: libavcodec/mips/fmtconvert_mips.c: patch does not apply Patch failed at 0001 mips: port optimizations to mips n64 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB In a rich man's house there is no place to spit but his face. -- Diogenes of Sinope signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 09/12] mips: port optimizations to mips n64
On Wed, 2015-03-04 at 11:52 +0100, Michael Niedermayer wrote: On Wed, Mar 04, 2015 at 10:10:15AM +, Nedeljko Babic wrote: LGTM seems this does not apply cleanly on HEAD Applying: mips: port optimizations to mips n64 error: patch failed: libavcodec/mips/acelp_filters_mips.c:82 error: libavcodec/mips/acelp_filters_mips.c: patch does not apply error: patch failed: libavcodec/mips/fmtconvert_mips.c:50 error: libavcodec/mips/fmtconvert_mips.c: patch does not apply Patch failed at 0001 mips: port optimizations to mips n64 Yeah, I'll resend it with my other small changes soon. James ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 02/12] mips/float_dsp: replace assembly with C implementations
On Wed, 2015-03-04 at 11:08 +, Nedeljko Babic wrote: The assembly versions have a few problems - They only work with mips32r2 enabled - They don't work on 64-bits - They're massive and complex So replace them with C implementations which solve these problems and let GCC magically optimize for different platforms. All the functions are manually unrolled 4 times (like the assembly code). With the addition of a few restrict keywords, the functions produce almost identical assembly to the original versions when compiled with gcc -O3. Since this code now uses no fpu assembly, drop the HAVE_MIPSFPU guard as well. All improvements of the C code should be put in generic C code so all architectures can benefit from them. The purpose of this code was to create optimizations for specific architecture. In this way optimizations for mips32r2 architecture are here even without tweaking configure line and even for older compilers. That's ok until you try to run it on an old MIPS processor and the default FFmpeg options cause lots of SIGILLs, but that's another discussion (and maybe nobody cares :/). By putting these optimizations under HAVE_MIPS32R2 problem with building mips64 should be resolved and this can be optimized for mips64 later if needed. I was thinking about just dropping this patch for the time being and porting a few bits to mips64 like in the other files (the code only uses the mips vi parts of mips32r2). The code only kept its performance when I unrolled the loops and used av_restrict. Strictly speaking you're not even supposed to use restrict if the arrays could be exactly equal (which is permitted in the contracts for some of these functions). James ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v3 2/2] libavformat/mxfenc: write package name metadata
On 2015-03-03 05:06, Mark Reid wrote:
+/*
+ * Returns the calculated length a local tag containing an utf-8
string as utf-16
+ */
+static uint64_t mxf_utf16_local_tag_length(const char *utf8_str)
+{
+return utf8_str? 4 + mxf_utf16len(utf8_str) * 2: 0;
Should return zero if mxf_utf16len()*2 65535.
+}
+
+/*
+ * Write a local tag containing an utf-8 string as utf-16
*/
static void mxf_write_local_tag_utf16(AVIOContext *pb, int tag, const
char *value)
{
-int i, size = strlen(value);
+int size = mxf_utf16len(value);
mxf_write_local_tag(pb, size*2, tag);
Same here. Check that size*2 65536. I don't want us outputting illegal
MXFs.
-for (i = 0; i size; i++)
-avio_wb16(pb, value[i]);
+avio_put_str16be(pb, value);
}
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 02/12] mips/float_dsp: replace assembly with C implementations
The assembly versions have a few problems - They only work with mips32r2 enabled - They don't work on 64-bits - They're massive and complex So replace them with C implementations which solve these problems and let GCC magically optimize for different platforms. All the functions are manually unrolled 4 times (like the assembly code). With the addition of a few restrict keywords, the functions produce almost identical assembly to the original versions when compiled with gcc -O3. Since this code now uses no fpu assembly, drop the HAVE_MIPSFPU guard as well. All improvements of the C code should be put in generic C code so all architectures can benefit from them. The purpose of this code was to create optimizations for specific architecture. In this way optimizations for mips32r2 architecture are here even without tweaking configure line and even for older compilers. By putting these optimizations under HAVE_MIPS32R2 problem with building mips64 should be resolved and this can be optimized for mips64 later if needed. -Nedeljko ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 02/12] mips/float_dsp: replace assembly with C implementations
On Thu, Feb 26, 2015 at 01:55:48PM +, James Cowgill wrote: On Thu, 2015-02-26 at 13:51 +, Derek Buitenhuis wrote: On 2/26/2015 1:42 PM, James Cowgill wrote: The assembly versions have a few problems - They only work with mips32r2 enabled - They don't work on 64-bits - They're massive and complex So replace them with C implementations which solve these problems and let GCC magically optimize for different platforms. All the functions are manually unrolled 4 times (like the assembly code). With the addition of a few restrict keywords, the functions produce almost identical assembly to the original versions when compiled with gcc -O3. Why have C implementations in the *MIPS* DSP code? That's silly. Hmm maybe a little. I was just worried that if I moved all the loop unrolling stuff into generic code it might go slower on other arches I haven't tested. i suspect unrolling tiny/small loops a bit helps on most architectures [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB During times of universal deceit, telling the truth becomes a revolutionary act. -- George Orwell signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC][PATCH] ffplay: factorize subtitle rendering code to a private filter
Le tridi 13 ventôse, an CCXXIII, Michael Niedermayer a écrit : yes and i really think we should do something about this IMHO API users should be able to define and register filters, codecs and formats For filters, now that they use AVFrame and the complex buffer management API is gone, it seems much more realistic. If someone were to try to work on it, though, I would appreciate they contact me to discuss some of the points in this mail: http://ffmpeg.org/pipermail/ffmpeg-devel/2014-October/164429.html It would be a waste to make public an API and deprecate it immediately. Concerning the current issue, maybe add to vf_subtitles an API to read the subtitles directly from a memory stream. That would still not be as good as real support for subtitles in filters, but at least it is not too ffplay-specific and could be used by other applications. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavformat/mxfdec: refactor reading strong ref array
On Sun, Mar 01, 2015 at 06:33:10PM -0800, Mark Reid wrote: hi, I was unsure whether this should be a function or a macro, I went with a function. i think functions are better, especially when you try to read a backtrace in gdb then again i think macros are better when you wonder why gcc didnt inline that speed critical and trivial fuction and made the whole software run half speed I think the first case applies more often though --- libavformat/mxfdec.c | 61 +++- 1 file changed, 22 insertions(+), 39 deletions(-) applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB There will always be a question for which you do not know the correct answer. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Patch review: av_dict: add support for empty values
On Tue, Mar 03, 2015 at 10:51:01PM -0400, Peter Cordes wrote: I've made some patches, and am finally getting around to sending them upstream. I'm trying out gmail for dealing with a high-traffic list like this one, instead of my usual mutt. Are the one-line-paragraph long lines it's probably going to produce an annoyance for anyone? I didn't notice anything about line lengths in emails in https://www.ffmpeg.org/developer.html I put in pull requests on github for 4 different branches. Should I send email to the list for each set of patches? Should I git-send-email, or is a mail with a URL for the github pull request enough? (Or is just a pull request enough? developer.html seems to say that sending a pull request is an alternative to mailing patches to the list.) I think what developer.html meant was that maintainers after they have made sure code is correct (though whatever means they consider neccessary and sufficient) contribute code by sending a mail to ffmpeg-devel or me asking for the code to be merged. people do contribute also through github style pull requests but i think everyone prefers the mailing lists for review and discussion of patches. so yes, please submit them with git send-email also ive taken a quick look at the first pull request - decimate-sad = av_pixelutils_get_sad_fn(3, 3, 0, decimate); // 8x8, not aligned on blocksize + decimate-sad = av_pixelutils_get_sad_fn(3, 3, 0, ctx); // 8x8, not aligned on blocksize i dont think this is needed though arguably it makes the code more consistent as the rest uses ctx. but both have a AVClass as first element and thus should work as av_log context So a commit message that point to consistency rather than correctness would be better about DECIMATE_PACKED_HACK, it would be best if this could be choosen at runtime by the user [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many that live deserve death. And some that die deserve life. Can you give it to them? Then do not be too eager to deal out death in judgement. For even the very wise cannot see all ends. -- Gandalf 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/2] acvodec/lipopenjpeg: Improve XYZ color space detection
On Wed, Mar 04, 2015 at 04:10:39PM +0200, Vilius Grigaliūnas wrote: On Wed, Mar 4, 2015 at 10:08 AM, Vilius Grigaliūnas vilius.grigaliu...@gmail.com wrote: On Wed, Mar 4, 2015 at 1:03 AM, Michael Niedermayer michae...@gmx.at wrote: this detects codestreams_profile0/p0_08.j2k as xyz, is this intended? No, this seems to be a regression. At the moment there is not enough information in openjpeg data structures (opj_image_t specifically) to reliably determine whether given image is is 12-bit XYZ or 12-bit RGB. I do not know how the best way to handle this case. Unless we can find some additional way to determine this, one of these cases will be broken. AFAIK there is no way to reliably determine color space just from naked J2K codestream. It basically boils down to a guess. In my opinion assuming that the value is 12-bit XYZ is just as valid as 12-bit RBG. I think in cases like this there should be a way to a pass pixel format value as input option. The way I understand jpeg2000 standard, XYZ interpretation would be more correct unless JP2 header or other source indicates otherwise. But I see how this would lead to mostly unexpected behavior in a lot of real-world cases. On Wed, Mar 4, 2015 at 12:10 PM, Michael Niedermayer michae...@gmx.at wrote: the best way would be to improve our jpeg2000 decoder so its at least as good as libopenjpeg then it can be used and it would have access to everything from the bitstream to determine the colorspace ... That would be great, but that does not mean that we should leave libopenjpeg decoder broken in specific cases (as in Digital Cinema which is one of the main use cases for JPEG200). I still see the utility in having it as it will have wider format support for the foreseeable future. i did not mean to suggest to leave libopenjpeg broken. Just that in every case our native decoders replaced external wrapers. If this happens with libopenjpeg too, all work on the wraper will be in vain as it will not be used anymore after some point. so i was imlpicitly thinking a bit ahead like waht will be there in the long term, lets concentrate work on that rather than the short term but by all means if you like improvig the libopenjpeg wraper, do so [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The greatest way to live with honor in this world is to be what we pretend to be. -- Socrates signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mov: write colr by default
On Wed, Mar 4, 2015 at 11:05 AM, Michael Niedermayer michae...@gmx.at wrote: On Wed, Mar 04, 2015 at 10:22:26AM +0100, Robert Krüger wrote: On Tue, Mar 3, 2015 at 9:27 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 09:24:17PM +0100, Robert Krüger wrote: On Tue, Mar 3, 2015 at 7:23 PM, Michael Niedermayer michae...@gmx.at wrote: On Tue, Mar 03, 2015 at 06:17:22PM +0100, Robert Krüger wrote: This is based on an earlier patch by Derek please mention this in the commit message OK, I will change that that never went in because it was argumented earlier that api breakage is not acceptable. However, that was more or less relaxed after Michael noted that the replaced flag had never been part of a release and since a number of people seem to agree, this is the better default, I am submitting this patch now, to have it in before the upcoming release. Let me know if that will be accepted and I will modify the respective fate tests as well. have you tested the generated mov and mp4 files with some common software packages ? checking random files on my disk it seems more than half the mov files contain a colr atom but i found just a single mp4 with a colr atom, so especially testing the compatibility of the mp4 files would be optimal before this is changed OK, I will do some tests with VLC, Quicktime, Final Cut, Final Cut X, Premiere and After Effects and maybe something else I find. thanks I tried an mp4 file with a colr atom with VLC, Quicktime, Final Cut X, Compressor, Premiere, After Effects and Adobe Media Encoder. None of those had any problems with the file and colors looked normal too. ok then please submit a patch that also updates fate here it is. mov_write_colr_by_default.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Patch review: av_dict: add support for empty values
On Wed, Mar 04, 2015 at 02:24:58PM +0100, Michael Niedermayer wrote:
On Wed, Mar 04, 2015 at 02:11:42PM +0100, Michael Niedermayer wrote:
On Tue, Mar 03, 2015 at 10:51:01PM -0400, Peter Cordes wrote:
[...]
Anyway, the av_dict change is the one that requires the most review, so
I'll make this email focus on that set of changes.
https://github.com/FFmpeg/FFmpeg/pull/118
pull req #3, patch #1 review
-char *ret = out, *end = out;
+char *ret = out, *end_quote = out;
why ?
also cosmetical changes should, if they are done, be in seperate
patches
-while (*p !strspn(p, term)) {
+while (*p !strchr(term, *p)) {
this should be in a seperate patch
+ret = av_realloc(ret, out - ret + 2);
+// if realloc fails to shrink, we're hosed anyway; just leak the old
buffer
return ret;
if realloc fails to shrink, the original unshrunk array should be
returned to avoid the leak and failure
ahh, i see you fix this in a later commit, this should be stashed
with the patch that would add the bug if it was pushed
and now i see you mentioned that in your mail, i guess replying to
the first part and then reviewing some of the code before reading
the rest of the mail was not really a great idea
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The worst form of inequality is to try to make unequal things equal.
-- Aristotle
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] GSOC2015
Hi Krishna! Am 04.03.15 um 07:10 schrieb krishna gandhi: Hi all I am Krishna Gupta undergraduate Electronics and Communication student at IIIT hyd. I have gone through this year ideas and project from org's GSOC page and one project that interest me a lot is MPEG-4 ALS encoder. I have little bit of theoretical idea about this from my undergraduate study. I have done courses like Digital signal Processing, speech signals and done a fair amount of c,c++ coding. I want to explore about this project more, so if someone can guide me with initial steps. nice to hear that! I will send you another private mail about getting started, soon. In the meantime, you might want to download and start reading the specs linked in our wiki pake (https://trac.ffmpeg.org/wiki/SponsoringPrograms/GSoC/2015). -Thilo ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Patch review: av_dict: add support for empty values
On Wed, Mar 04, 2015 at 02:11:42PM +0100, Michael Niedermayer wrote:
On Tue, Mar 03, 2015 at 10:51:01PM -0400, Peter Cordes wrote:
[...]
Anyway, the av_dict change is the one that requires the most review, so
I'll make this email focus on that set of changes.
https://github.com/FFmpeg/FFmpeg/pull/118
pull req #3, patch #1 review
-char *ret = out, *end = out;
+char *ret = out, *end_quote = out;
why ?
also cosmetical changes should, if they are done, be in seperate
patches
-while (*p !strspn(p, term)) {
+while (*p !strchr(term, *p)) {
this should be in a seperate patch
+ret = av_realloc(ret, out - ret + 2);
+// if realloc fails to shrink, we're hosed anyway; just leak the old
buffer
return ret;
if realloc fails to shrink, the original unshrunk array should be
returned to avoid the leak and failure
ahh, i see you fix this in a later commit, this should be stashed
with the patch that would add the bug if it was pushed
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
You can kill me, but you cannot change the truth.
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Patch review: av_dict: add support for empty values
On Tue, Mar 03, 2015 at 10:51:01PM -0400, Peter Cordes wrote:
[...]
Anyway, the av_dict change is the one that requires the most review, so
I'll make this email focus on that set of changes.
https://github.com/FFmpeg/FFmpeg/pull/118
pull req #3, patch #1 review
-char *ret = out, *end = out;
+char *ret = out, *end_quote = out;
why ?
also cosmetical changes should, if they are done, be in seperate
patches
-while (*p !strspn(p, term)) {
+while (*p !strchr(term, *p)) {
this should be in a seperate patch
+ret = av_realloc(ret, out - ret + 2);
+// if realloc fails to shrink, we're hosed anyway; just leak the old
buffer
return ret;
if realloc fails to shrink, the original unshrunk array should be
returned to avoid the leak and failure
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
No snowflake in an avalanche ever feels responsible. -- Voltaire
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] acvodec/lipopenjpeg: Improve XYZ color space detection
On Wed, Mar 4, 2015 at 10:08 AM, Vilius Grigaliūnas vilius.grigaliu...@gmail.com wrote: On Wed, Mar 4, 2015 at 1:03 AM, Michael Niedermayer michae...@gmx.at wrote: this detects codestreams_profile0/p0_08.j2k as xyz, is this intended? No, this seems to be a regression. At the moment there is not enough information in openjpeg data structures (opj_image_t specifically) to reliably determine whether given image is is 12-bit XYZ or 12-bit RGB. I do not know how the best way to handle this case. Unless we can find some additional way to determine this, one of these cases will be broken. AFAIK there is no way to reliably determine color space just from naked J2K codestream. It basically boils down to a guess. In my opinion assuming that the value is 12-bit XYZ is just as valid as 12-bit RBG. I think in cases like this there should be a way to a pass pixel format value as input option. The way I understand jpeg2000 standard, XYZ interpretation would be more correct unless JP2 header or other source indicates otherwise. But I see how this would lead to mostly unexpected behavior in a lot of real-world cases. On Wed, Mar 4, 2015 at 12:10 PM, Michael Niedermayer michae...@gmx.at wrote: the best way would be to improve our jpeg2000 decoder so its at least as good as libopenjpeg then it can be used and it would have access to everything from the bitstream to determine the colorspace ... That would be great, but that does not mean that we should leave libopenjpeg decoder broken in specific cases (as in Digital Cinema which is one of the main use cases for JPEG200). I still see the utility in having it as it will have wider format support for the foreseeable future. Native jpeg2000 only uses xyz pixels if it detects that the file in Digital Cinema profile (RSiz = 3 or RSiz = 4). While libopenjpeg does not expose this, it is also stored in MXF container which is the standard wrapper for DC j2k streams. That means we can detect xyz pixels in avformat/mxfdec and pass it to decoder. Which might be an even better solution. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Patch review: av_dict: add support for empty values
On Tue, Mar 03, 2015 at 10:51:01PM -0400, Peter Cordes wrote: I've made some patches, and am finally getting around to sending them upstream. I'm trying out gmail for dealing with a high-traffic list like this one, instead of my usual mutt. Are the one-line-paragraph long lines it's probably going to produce an annoyance for anyone? I didn't notice anything about line lengths in emails in https://www.ffmpeg.org/developer.html I put in pull requests on github for 4 different branches. Should I send email to the list for each set of patches? Should I git-send-email, or is a mail with a URL for the github pull request enough? (Or is just a pull request enough? developer.html seems to say that sending a pull request is an alternative to mailing patches to the list.) Anyway, the av_dict change is the one that requires the most review, so I'll make this email focus on that set of changes. https://github.com/FFmpeg/FFmpeg/pull/118 Currently, -x264-params nocabac:ref=4:ssim isn't supported. You have to say nocabac=1:ref=4:ssim=1 (or cabac=0...). (and yes, I know about -preset, but superfast no-cabac is useful... anyway.) Also, libx265.c is very minimal, and needs a lot of things passed in -x265-params. I didn't fix that, but did make it support keys with no value. libx264 has two different ways of passing option strings (-x264-params and -x264opts), due to avconv gratuitously renaming things. The insane part is that the two strings were parsed differently. One was handled by av_dict, the other with some custom code that didn't support quoting. In my x26x-opts branch (which follows the av_dict branch), I add both option strings to the same av_dict. (x264opts and then x264-params, in that order, regardless of order on the command line, same as before). I don't think it's ever useful to use the same key multiple times. Tune only works as -tune ssim, not in x264-params, and you can use a comma-separated list for -tune animation,ssim. There was a previous attempt at keys with no value that didn't get merged: http://ffmpeg.org/pipermail/ffmpeg-devel/2013-July/146329.html I should borrow the idea of adding a new flag for supporting NULL as a value, instead of the cumbersome empty-string method I went with to minimize changes to some parts of the code. Having a flag would make sure the behaviour didn't change for any callers that weren't expecting it. (i.e. it would still reject key:key). So are any of these changes useful? I assume I'll end up redoing the commits anyway after a review, so I didn't try to make a final ready-for-commit version yet. (For that, I would merge the bugfix changes into my original buggy commit.) For review, I'd suggest looking at the diff of master:av_dict. The 3 commits after that changing libx264.c and libx265.c should each stand on their own. (And actually the libx264.c one should be split into a couple commits.) Anyway, I wanted to post first and get some feedback before spending more time on it. to awnser your question i definitly think the changes are usefull -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Old school: Use the lowest level language in which you can solve the problem conveniently. New school: Use the highest level language in which the latest supercomputer can solve the problem without the user falling asleep waiting. 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/2] acvodec/lipopenjpeg: Improve XYZ color space detection
On Wed, Mar 4, 2015 at 4:48 PM, Michael Niedermayer michae...@gmx.at wrote: i did not mean to suggest to leave libopenjpeg broken. It is not technically broken, it just relies on guessing in some cases which might not always be correct. It think there should be a warning that this is happening and a way to override it. I had made a patch that allows to set pix_fmt via codec options. But I found out that the codec gets called two times, first without options to detect format and then with options to perform decoding. That means that the pixel format changes after the decoding starts. Although it works I do not find it very elegant. Anyway, I will try to implement pixel format detection in MXF decoder and that should solve the issue for my use case. Just that in every case our native decoders replaced external wrapers. If this happens with libopenjpeg too, all work on the wraper will be in vain as it will not be used anymore after some point. so i was imlpicitly thinking a bit ahead like waht will be there in the long term, lets concentrate work on that rather than the short term but by all means if you like improvig the libopenjpeg wraper, do so I thought this would be a quick fix, but the reality turned out to be a bit more complicated. :) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
