[FFmpeg-devel] What is the correct way to read Avid ACLR atom?

[Kevin Wheatley]

Hi,

I've been looking at what the appropriate place and method to read in
the ACLR atom placed in the Avid codecs so that we can automatically
set the color_range.

From my reading of the code the mov.c code the parse table shunts the
reading of the atom to mov_read_avid(), which in turn calls
mov_read_extradata() which leaves the data in the codec extradata
buffer.

The issue would be at which point is the preferred point to look into
that buffer and set the colour_range, should it be done in the
specific codec's file in libavcodec (I'm only interested in DNxHD but
I assume the other Avid codecs might need it), or because of this
duplication should the change be made in libavformat/mov.c somewhere
like mov_read_avid() in a similar way to the mov_read_ares()
functions?

Thanks

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

Re: [FFmpeg-devel] What is the correct way to read Avid ACLR atom?

[Kevin Wheatley]

On Thu, Feb 5, 2015 at 3:55 PM, wm4 nfx...@googlemail.com wrote:
 Seeing how the patch actually turned out, I'd say it should be in the
 decoder. It's not necessarily mp4 specific, or is it?

In this case I'm exporting the QuickTimes from an Avid Media Composer
so they are DNxHD in .mov, and in .mp4 with both 'RGB' and '709'
settings in Avid speak. From reading the ffmpeg code it looks like
other 'Avid' codecs also use similar atoms inside .mov derivatives,
but I'm not personally aware if any other container format has them.

My question about code style as regards the extradata was me thinking,
if I have read the data and done something with it, it isn't really
'extra' anymore but I'm not very familiar with ffmpeg's coding style
and architecture.

Putting this function in the avformat side of the code allows any
codec to pick it up and looked to be the most contained change, but it
potentially does change the image processing as now it can determine
the encoding range things may get handled differently - this is part
of my goal which is to correctly transport the colour metadata across
a number of different codecs and containers so that appropriate
conversions can be applied.

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

Re: [FFmpeg-devel] What is the correct way to read Avid ACLR atom?

[Kevin Wheatley]

On Thu, Feb 5, 2015 at 4:24 PM, Carl Eugen Hoyos ceho...@ag.or.at wrote:
 Kevin Wheatley kevin.j.wheatley at gmail.com writes:

 +int ret = mov_read_avid(c, pb, atom);
 // should we do this or read the atom directly
 using avio_*() and not store it in extradata?

 Not storing the atom in extradata would break
 remuxing.

ah, so the answer to my comment is quite clear now

 Did you test your patch? If it works, what
 advantage would the decoder patch(es) have?
 There would be a few iirc, no?


I assume there would need to be put in different files, as an example
of why I thought the mov decoder would be a good choice is that I am
not sure if DNxHD when wrapped in MXF has a similar choice of encoding
ranges - by default the typical Avid work flow is to use '709'
encoding range, but sometimes our clients want 'RGB'.

As to testing the patch, yes in a few ways, but not exhaustively -
mostly verifying ffprobe correctly outputs the encoding range on a
variety of files as well as outputting PNGs with the expected range
expansion on '709' content, but none on 'RGB' via ffmpeg -i
qt_dnxhd.mov blah.%04d.png.

I haven't looked at anything more fancy, although I did note that -c
copy does not propagate the colour_range but I've not looked at why.

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

[FFmpeg-devel] compile failure with -DDEBUG of HEAD

[Kevin Wheatley]

Whilst compiling with -DDEBUG I get the following...

libavformat/rtpdec_h264.c: In function 'h264_handle_packet_stap_a':
libavformat/rtpdec_h264.c:208: error: 'data' undeclared (first use in
this function)
libavformat/rtpdec_h264.c:208: error: (Each undeclared identifier is
reported only once
libavformat/rtpdec_h264.c:208: error: for each function it appears in.)
libavformat/rtpdec_h264.c: In function 'h264_handle_packet_fu_a':
libavformat/rtpdec_h264.c:259: error: 'data' undeclared (first use in
this function)

Looks like passing in the needed context to from the calling functions
would work,

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

Re: [FFmpeg-devel] compile failure with -DDEBUG of HEAD

[Kevin Wheatley]

This time with patch...

On Mon, Feb 16, 2015 at 4:58 PM, Kevin Wheatley
kevin.j.wheat...@gmail.com wrote:
 Whilst compiling with -DDEBUG I get the following...

 libavformat/rtpdec_h264.c: In function 'h264_handle_packet_stap_a':
 libavformat/rtpdec_h264.c:208: error: 'data' undeclared (first use in
 this function)
 libavformat/rtpdec_h264.c:208: error: (Each undeclared identifier is
 reported only once
 libavformat/rtpdec_h264.c:208: error: for each function it appears in.)
 libavformat/rtpdec_h264.c: In function 'h264_handle_packet_fu_a':
 libavformat/rtpdec_h264.c:259: error: 'data' undeclared (first use in
 this function)

 Looks like passing in the needed context to from the calling functions
 would work,

 Kevin
diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c
index 89053ef..24b701c 100644
--- a/libavformat/rtpdec_h264.c
+++ b/libavformat/rtpdec_h264.c
@@ -177,7 +177,7 @@ static int sdp_parse_fmtp_config_h264(AVFormatContext *s,
 return 0;
 }
 
-static int h264_handle_packet_stap_a(AVFormatContext *ctx, AVPacket *pkt,
+static int h264_handle_packet_stap_a(AVFormatContext *ctx, PayloadContext *data, AVPacket *pkt,
  const uint8_t *buf, int len)
 {
 int pass = 0;
@@ -234,7 +234,7 @@ static int h264_handle_packet_stap_a(AVFormatContext *ctx, AVPacket *pkt,
 return 0;
 }
 
-static int h264_handle_packet_fu_a(AVFormatContext *ctx, AVPacket *pkt,
+static int h264_handle_packet_fu_a(AVFormatContext *ctx, PayloadContext *data, AVPacket *pkt,
const uint8_t *buf, int len)
 {
 uint8_t fu_indicator, fu_header, start_bit, nal_type, nal;
@@ -308,7 +308,7 @@ static int h264_handle_packet(AVFormatContext *ctx, PayloadContext *data,
 buf++;
 len--;
 // first we are going to figure out the total size
-result = h264_handle_packet_stap_a(ctx, pkt, buf, len);
+result = h264_handle_packet_stap_a(ctx, data, pkt, buf, len);
 break;
 
 case 25:   // STAP-B
@@ -322,7 +322,7 @@ static int h264_handle_packet(AVFormatContext *ctx, PayloadContext *data,
 break;
 
 case 28:   // FU-A (fragmented nal)
-result = h264_handle_packet_fu_a(ctx, pkt, buf, len);
+result = h264_handle_packet_fu_a(ctx, data, pkt, buf, len);
 break;
 
 case 30:   // undefined
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat: Add simple ACLR atom reading to set the color range

[Kevin Wheatley]

So reading through the comments in this thread, it looks to me like
some of the problems come from the use of the mov_read_extradata()
function, which has more logic than the name implies, in particular it
will successfully return even if it did not read the atom into the
extradata, so if i just directly read the atom, what will break? I
could do the ugly thing of reading it and then filling the extradata
(or modify the reading function to better communicate to the callers
if the data was read correctly).

The atom could also not be in the extradata but does it have to be?

The movenc.c writer of the atom doesn't always use the extradata - for
DNxHD relies on the colour_range of the track's encoder directly and
doesn't appear to write the extradata (calls mov_write_avid_tag()),
for other codecs it uses mov_write_extradata_tag() (for
AV_CODEC_ID_AVU and AV_CODEC_ID_SVQ3).

This suggests that for DNxHD I don't need it in the extradata to
preserve behaviour, or am I missing something in the code (something
in the use as a library for instance)?

Thanks

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

Re: [FFmpeg-devel] [PATCH] avformat: Add simple ACLR atom reading to set the color range

[Kevin Wheatley]

Oops, I missed that when I allowed the atom to be added to the end of
the extradata, updated.

I'm not sure if logging as an error is appropriate in the case when
the extradata is not large enough, for this to occur the file will
have been truncated somehow and the mov_read_extradata function will
have logged a warning and returned 0. Which at first glance seams
wrong - should mov_read_extradata return something other than 0 in
this case?

Either way it does mean the code could read the wrong value, but
should not crash with the guard in place.

Thanks

Kevin
From c48f8f2d74562e0e3ee9e6d496fc6a1c4320db14 Mon Sep 17 00:00:00 2001
From: Kevin Wheatley kevin.j.wheat...@gmail.com
Date: Tue, 17 Feb 2015 11:47:47 +
Subject: [PATCH] Add simple ACLR atom reading to set the color range of the incomming
 track for codec's like DNxHD that utilise AVID's proprietary atom.

Added paranoia check for memory reallocation weirdness that might
occur (should never trigger but be defensive)

Signed-off-by: Kevin Wheatley kevin.j.wheat...@gmail.com
---
 libavformat/mov.c |   36 +++-
 1 files changed, 35 insertions(+), 1 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 6d2262a..28a140a 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1178,6 +1178,40 @@ static int mov_read_ares(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 return mov_read_avid(c, pb, atom);
 }
 
+static int mov_read_aclr(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+int ret = mov_read_avid(c, pb, atom);
+
+if (!ret  c-fc-nb_streams = 1) {
+AVCodecContext *codec = c-fc-streams[c-fc-nb_streams-1]-codec;
+if (atom.size == 16) {
+if (codec-extradata_size = atom.size + 8) {
+/* This assumes the atom will be at the end of the extradata */
+const uint8_t range_value = codec-extradata[codec-extradata_size - 5];
+switch (range_value) {
+case 1:
+codec-color_range = AVCOL_RANGE_MPEG;
+break;
+case 2:
+codec-color_range = AVCOL_RANGE_JPEG;
+break;
+default:
+av_log(c, AV_LOG_WARNING, unknown aclr value (%d)\n, range_value);
+break;
+}
+av_dlog(c, color_range: %PRIu8\n, codec-color_range);
+} else {
+  /* For some reason the whole atom was not added to the extradata */
+  av_log(c, AV_LOG_ERROR, aclr not decoded - incomplete extradata size %d\n, codec-extradata_size);
+}
+} else {
+av_log(c, AV_LOG_WARNING, aclr not decoded - unexpected size %ld\n, atom.size);
+}
+}
+
+return ret;
+}
+
 static int mov_read_svq3(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
 return mov_read_extradata(c, pb, atom, AV_CODEC_ID_SVQ3);
@@ -3390,7 +3424,7 @@ static int mov_read_free(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 }
 
 static const MOVParseTableEntry mov_default_parse_table[] = {
-{ MKTAG('A','C','L','R'), mov_read_avid },
+{ MKTAG('A','C','L','R'), mov_read_aclr },
 { MKTAG('A','P','R','G'), mov_read_avid },
 { MKTAG('A','A','L','P'), mov_read_avid },
 { MKTAG('A','R','E','S'), mov_read_ares },
-- 
1.7.1

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

Re: [FFmpeg-devel] [PATCH] avformat: Add simple ACLR atom reading to set the color range

[Kevin Wheatley]

V3 of the patch, I've attempted to include the general comments from the thread.

New to this version, I've reworked the function that reads the atom
into the extradata into one which calls 2 helper functions (one to
realloc, one to read), I've then reused these functions to read the
ACLR atom reliably.

My ACLR reading function now calls these functions directly rather
than via the mov_read_avid() function, this means that the atom will
be used to set the range no matter which codec is used for the track
(in the current code the mov_read_avid() function limits it to a
restricted set pf codecs).

I have not changed how the ACLR is used in the writer as it will still
be found in the extradata buffer, this does mean that there maybe
cases of 'none avid' codecs that now have the atom and when ouput
could have modified behavior  but I've never seen one of these files
to test with.

Thanks for any comments,

Kevin
From fe6216aec8592baaf40edaa61a73321161548009 Mon Sep 17 00:00:00 2001
From: Kevin Wheatley kevin.j.wheat...@gmail.com
Date: Thu, 19 Feb 2015 11:08:14 +
Subject: [PATCH] Add simple ACLR atom reading to set the color range of the incomming
 track for codec's like DNxHD that utilise AVID's proprietary atom.

On input ACLR will be used to set colour range no matter which codec
it is associated with.
No change for when it will be output.

Rework mov_read_extradata function to allow detection of truncated
atom reads by callers.

Signed-off-by: Kevin Wheatley kevin.j.wheat...@gmail.com
---
 libavformat/mov.c |  108 +---
 1 files changed, 85 insertions(+), 23 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 6d2262a..7718d8e 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1079,42 +1079,63 @@ static int mov_read_fiel(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 return 0;
 }
 
+static int mov_realloc_extradata(AVCodecContext *codec, MOVAtom atom)
+{
+int err = 0;
+uint64_t size = (uint64_t)codec-extradata_size + atom.size + 8 + FF_INPUT_BUFFER_PADDING_SIZE;
+if (size  INT_MAX || (uint64_t)atom.size  INT_MAX)
+return AVERROR_INVALIDDATA;
+if ((err = av_reallocp(codec-extradata, size))  0) {
+codec-extradata_size = 0;
+return err;
+}
+codec-extradata_size = size - FF_INPUT_BUFFER_PADDING_SIZE;
+return 0;
+}
+
+/* Read a whole atom into the extradata return the size of the atom read, possibly truncated if != atom.size */
+static int64_t mov_read_atom_into_extradata(MOVContext *c, AVIOContext *pb, MOVAtom atom,
+AVCodecContext *codec, uint8_t *buf)
+{
+int64_t result = atom.size;
+int err;
+
+AV_WB32(buf, atom.size + 8);
+AV_WL32(buf + 4, atom.type);
+err = avio_read(pb, buf + 8, atom.size);
+if (err  0) {
+return err;
+} else if (err  atom.size) {
+av_log(c-fc, AV_LOG_WARNING, truncated extradata\n);
+codec-extradata_size -= atom.size - err;
+result = err;
+}
+memset(buf + 8 + err, 0, FF_INPUT_BUFFER_PADDING_SIZE);
+return result;
+}
+
 /* FIXME modify qdm2/svq3/h264 decoders to take full atom as extradata */
 static int mov_read_extradata(MOVContext *c, AVIOContext *pb, MOVAtom atom,
   enum AVCodecID codec_id)
 {
 AVStream *st;
-uint64_t size;
-uint8_t *buf;
+uint64_t original_size;
 int err;
 
 if (c-fc-nb_streams  1) // will happen with jp2 files
 return 0;
-st= c-fc-streams[c-fc-nb_streams-1];
+st = c-fc-streams[c-fc-nb_streams-1];
 
 if (st-codec-codec_id != codec_id)
 return 0; /* unexpected codec_id - don't mess with extradata */
 
-size= (uint64_t)st-codec-extradata_size + atom.size + 8 + FF_INPUT_BUFFER_PADDING_SIZE;
-if (size  INT_MAX || (uint64_t)atom.size  INT_MAX)
-return AVERROR_INVALIDDATA;
-if ((err = av_reallocp(st-codec-extradata, size))  0) {
-st-codec-extradata_size = 0;
+original_size = st-codec-extradata_size;
+err = mov_realloc_extradata(st-codec, atom);
+if (err)
 return err;
-}
-buf = st-codec-extradata + st-codec-extradata_size;
-st-codec-extradata_size= size - FF_INPUT_BUFFER_PADDING_SIZE;
-AV_WB32(   buf, atom.size + 8);
-AV_WL32(   buf + 4, atom.type);
-err = avio_read(pb, buf + 8, atom.size);
-if (err  0) {
-return err;
-} else if (err  atom.size) {
-av_log(c-fc, AV_LOG_WARNING, truncated extradata\n);
-st-codec-extradata_size -= atom.size - err;
-}
-memset(buf + 8 + err, 0, FF_INPUT_BUFFER_PADDING_SIZE);
-return 0;
+
+err =  mov_read_atom_into_extradata(c, pb, atom, st-codec,  st-codec-extradata + original_size);  
+return 0; // Note: this is the original behavior to ignore truncation.
 }
 
 /* wrapper functions for reading ALAC/AVS/MJPEG/MJPEG2000 extradata atoms only for those codecs */
@@ -1178,6 +1199,47

[FFmpeg-devel] Gaining access to a encoder context in avformat?

[Kevin Wheatley]

Hi,

I realise this potentially breaks through lots of API boundaries/good
taste, but I've noticed a case where the movenc.c code should really
ask the specific encoder context (DNXHDEncContext) for some of it's
fields to ensure that the atoms written have a consistent setting with
what the codec did.

Is this something 'allowed'? How best should I access it?

thanks

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

Re: [FFmpeg-devel] Gaining access to a encoder context in avformat?

[Kevin Wheatley]

On Fri, Feb 20, 2015 at 11:44 AM, Michael Niedermayer michae...@gmx.at wrote:
 If you start with a existing movie and copy the packets one by one
 there is no decoder and no encoder, so no dnxencoder structure

 if you want to put some value in the avid atom which are stored in the
 bitstream/packets of dnxhd and not in the generic data structures
 like width/height would be then you have to extract these from the
 dnxhd bitstream one way or another because thats the only thing thats
 there in the memory of your machine. I dont know what exact information
 you want/need from the dnxhd encoder ...


hmm, maybe I should just say that copy is not supported for DNxHD
files then as that sounds a pain to deal with, or I could put better
check in the arbitrary buffer offsets the mov_write_avid_tag function
uses to access track-vos_data, which is what sent me down the
direction of trying to access the encoder data. Is there a sensible
function I can use to find a specific atom within the buffer?

On a related note when doing a -c copy is it supposed to preserve
color range (trc, space, primaries) in the output if it is set on the
input/command line?

Thanks

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

Re: [FFmpeg-devel] Gaining access to a encoder context in avformat?

[Kevin Wheatley]

Please excuse my newbie knowledge of the code base BTW...

I've just done a simple test

ffmpeg -color_range mpeg -i test_charts/test_encoding.tif -c dnxhd -vb
115M  /quicktimes/ffmpeg_test.mov

During this the vos_data contains...

00 00 02 80 01 01 80 A0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 04 38 07 80 00 04 38 00 00 38 88 ...

Which looks to me like the start of the VC3/DNxHD bit stream and in
this case the code puts valid data in the header atoms.

If I then

fmpeg -i quicktimes/ffmpeg_test.mov -c copy quicktimes/ffmpeg_test_copy.mov

the vos_data instead contains

00 00 00 18 41 43 4C 52 41 43 4C 52 30 30 30 31 00 00 00 01 00 00 00
00 00 00 00 18 41 50 52 47 41 50 52 47 ...

which is the  start of the Avid atoms from the incomming Quicktime.

So I could peak into the stream and 'guess' based on the content being
0x00, 0x00, 0x02, 0x80, 0x01  that we are encoding and can trust the
contents, else I could search through looking for the atom via the
string ARESARES and then having located it I could assume to use
that. The only oddball case is if it is not found, at that point the
code needs to know if it is interlaced as well as the avid codec
identifier, or maybe that is the point at which we 'give up' and fail
with unsupported?

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

Re: [FFmpeg-devel] Gaining access to a encoder context in avformat?

[Kevin Wheatley]

Here is the kind of thing that this looks like... This is definitely
NOT a patch :-)

On Fri, Feb 20, 2015 at 1:22 PM, Michael Niedermayer michae...@gmx.at wrote:
 On Fri, Feb 20, 2015 at 12:35:59PM +, Kevin Wheatley wrote:
 Please excuse my newbie knowledge of the code base BTW...

 I've just done a simple test

 ffmpeg -color_range mpeg -i test_charts/test_encoding.tif -c dnxhd -vb
 115M  /quicktimes/ffmpeg_test.mov

 During this the vos_data contains...

 00 00 02 80 01 01 80 A0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 04 38 07 80 00 04 38 00 00 38 88 ...

 Which looks to me like the start of the VC3/DNxHD bit stream and in
 this case the code puts valid data in the header atoms.

 If I then

 fmpeg -i quicktimes/ffmpeg_test.mov -c copy quicktimes/ffmpeg_test_copy.mov

 the vos_data instead contains

 00 00 00 18 41 43 4C 52 41 43 4C 52 30 30 30 31 00 00 00 01 00 00 00
 00 00 00 00 18 41 50 52 47 41 50 52 47 ...

 which is the  start of the Avid atoms from the incomming Quicktime.

 So I could peak into the stream and 'guess' based on the content being
 0x00, 0x00, 0x02, 0x80, 0x01  that we are encoding and can trust the
 contents, else I could search through looking for the atom via the
 string ARESARES and then having located it I could assume to use
 that. The only oddball case is if it is not found, at that point the
 code needs to know if it is interlaced as well as the avid codec
 identifier, or maybe that is the point at which we 'give up' and fail
 with unsupported?

 if you need to put 2 bits from a fixed location of the first packet
 into some avid atoms its probably easiest to read them straight out of
 the first packet into some new field in a struct and then use that
 when writing the atoms.

 IIUC theres no generic field in AVStream, AVCodecContext or elsewhere
 that holds this information. If there is, setting this field from the
 encoder/demuxer and using it would be the more correct path

 extracting the data from vos_data seems trickier

 It would also be possible to add a first_packet field where vos_data
 is and always initialize it to the first packet and use that instead

 I sure do not fully know all the details of the issue so i might be
 missing something

 [...]

 --
 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

 There will always be a question for which you do not know the correct answer.

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

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index a72f84e..dc5a7b4 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1033,6 +1033,10 @@ static int mov_write_hvcc_tag(AVIOContext *pb, MOVTrack 
*track)
 static int mov_write_avid_tag(AVIOContext *pb, MOVTrack *track)
 {
 int i;
+int interlaced = 0;
+int cid = 0;
+uint32_t temp;
+
 avio_wb32(pb, 24); /* size */
 ffio_wfourcc(pb, ACLR);
 ffio_wfourcc(pb, ACLR);
@@ -1056,10 +1060,43 @@ static int mov_write_avid_tag(AVIOContext *pb, MOVTrack 
*track)
 ffio_wfourcc(pb, ARES);
 ffio_wfourcc(pb, ARES);
 ffio_wfourcc(pb, 0001);
-avio_wb32(pb, AV_RB32(track-vos_data + 0x28)); /* dnxhd cid, some id ? */
+
+if (track-vos_data  track-vos_len  0x29) { 
+if (track-vos_data[0] == 0x00 
+track-vos_data[1] == 0x00 
+track-vos_data[2] == 0x02 
+track-vos_data[3] == 0x80 
+(track-vos_data[4] == 0x01 ||track-vos_data[4] == 0x02)) {
+/* looks like a DNxHD bit stream */
+interlaced = (track-vos_data[5]  2);
+cid = AV_RB32(track-vos_data + 0x28);
+} else if (track-vos_len  120) { /* big enough to contain the atom */
+for (i = 0; i  (track-vos_len - 4); i += 4) {
+temp = AV_RL32(track-vos_data + i);
+if ((AV_RL32(track-vos_data + i) == MKTAG('A', 'R', 'E', 
'S')) 
+(AV_RL32(track-vos_data + i + 4) == MKTAG('A', 'R', 'E', 
'S')))
+break;
+}
+if (i  (track-vos_len - 12))
+{
+interlaced = track-enc-field_order  AV_FIELD_PROGRESSIVE;
+cid = AV_RB32(track-vos_data + i + 12);
+} else {
+// couldn't find it in the buffer
+printf(couldn't find it\n);
+}
+}
+} else {
+  printf(buffer too small\n);
+  // log some error not enough buffer to contain the info?
+  // return non 0?
+}
+
+
+avio_wb32(pb, cid); /* dnxhd cid, some id ? */
 avio_wb32(pb, track-enc-width);
 /* values below are based on samples created with quicktime and avid 
codecs */
-if (track-vos_data[5]  2) { // interlaced
+if (interlaced) {
 avio_wb32(pb, track-enc-height / 2);
 avio_wb32(pb, 2); /* unknown */
 avio_wb32(pb, 0); /* unknown

Re: [FFmpeg-devel] Why is writing the colr atom not the default in the mov muxer?

[Kevin Wheatley]

On Fri, Feb 20, 2015 at 1:30 PM, Robert Krüger krue...@lesspain.de wrote:
 if I read the code correctly, the colr atom is only written in the mov
 muxer if the flag write_colr is specified. Was that behaviour chosen to
 have better backward compatibility or is there another reason not to write
 this standard atom by default?

I chose that way to preserve the older behaviour, as it can change how
files will be interpreted.

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

Re: [FFmpeg-devel] Gaining access to a encoder context in avformat?

[Kevin Wheatley]

On Fri, Feb 20, 2015 at 2:14 PM, Michael Niedermayer michae...@gmx.at wrote:
 On Fri, Feb 20, 2015 at 01:48:55PM +, Kevin Wheatley wrote:
 Here is the kind of thing that this looks like... This is definitely
 NOT a patch :-)

 i dont understand this patch

there are at least two of us.

 please explain why you use vos_data
 it can be the first packet and you seem to implement that case
 but it is not always the first packet and you seem to add alot of
 code to handle this. But if the data is in the dnxhd packets
 using that always and not vos_data seems much easier
 what am i missing ?

probably nothing, I've only tried to find a small change to the code,
and to localise the effect to the function so as to minimise the
effect of the change - I really don't have a full enough grasp of the
code base to do otherwise.

 or one could ask the other way around why vos data is set
 inconsistently, is there a case where setting it to the first packet
 does not work?


I'm simply showing how the current contents of vos_data can be used,
like you say it would be nice if it was always one or the other, but
for whatever reason the copy case puts the atoms in there, which is
why the current code without the patch generates incorrect atoms when
copy is used on DNxHD files as the code that writes the new atom
expects the bitstream, all I did was try a quick detection of what the
contents are and use them appropriately. to see if the files would
then be fixed.

In my case I'm unable to read the files in The Foundry's Nuke if I
'copy' them via ffmpeg because the atom was written incorrectly, but
with the diff applied then the copied files will load.

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

[FFmpeg-devel] [PATCH] avformat: Add simple ACLR atom reading to set the color range

[Kevin Wheatley]

 Add simple ACLR atom reading to set the color range of the incomming
 track for codec's like DNxHD that utilise AVID's proprietary atom.

Note: for this to work with ffmpeg generated DNxHD QuickTime files you
need to also use my other patch to prevent ffmpeg generating 'corrupt'
files.
From 561db6b347bed1f60131c3eb2bebe890a402ad63 Mon Sep 17 00:00:00 2001
From: Kevin Wheatley kevin.j.wheat...@gmail.com
Date: Tue, 17 Feb 2015 09:15:06 +
Subject: [PATCH] Add simple ACLR atom reading to set the color range of the incomming
 track for codec's like DNxHD that utilise AVID's proprietary atom.


Signed-off-by: Kevin Wheatley kevin.j.wheat...@gmail.com
---
 libavformat/mov.c |   32 +++-
 1 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 6d2262a..0d4b0cf 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1178,6 +1178,36 @@ static int mov_read_ares(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 return mov_read_avid(c, pb, atom);
 }
 
+static int mov_read_aclr(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+int ret = mov_read_avid(c, pb, atom);
+
+if (!ret  c-fc-nb_streams = 1) {
+if (atom.size == 16) {
+AVCodecContext *codec = c-fc-streams[c-fc-nb_streams-1]-codec;
+
+/* This assumes the atom will be at the end of the extradata */
+const uint8_t range_value = codec-extradata[codec-extradata_size - 5];
+switch (range_value) {
+case 1:
+codec-color_range = AVCOL_RANGE_MPEG;
+break;
+case 2:
+codec-color_range = AVCOL_RANGE_JPEG;
+break;
+default:
+av_log(c, AV_LOG_WARNING, unknown aclr value (%d)\n, range_value);
+break;
+}
+av_dlog(c, color_range: %PRIu8\n, codec-color_range);
+} else {
+av_log(c, AV_LOG_WARNING, aclr not decoded - unexpected size %ld\n, atom.size);
+}
+}
+
+return ret;
+}
+
 static int mov_read_svq3(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
 return mov_read_extradata(c, pb, atom, AV_CODEC_ID_SVQ3);
@@ -3390,7 +3420,7 @@ static int mov_read_free(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 }
 
 static const MOVParseTableEntry mov_default_parse_table[] = {
-{ MKTAG('A','C','L','R'), mov_read_avid },
+{ MKTAG('A','C','L','R'), mov_read_aclr },
 { MKTAG('A','P','R','G'), mov_read_avid },
 { MKTAG('A','A','L','P'), mov_read_avid },
 { MKTAG('A','R','E','S'), mov_read_ares },
-- 
1.7.1

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

Re: [FFmpeg-devel] [PATCH] avformat: Add simple ACLR atom reading to set the color range

[Kevin Wheatley]

On Tue, Feb 17, 2015 at 12:25 PM, Michael Niedermayer michae...@gmx.at wrote:
 if the codec id doesnt match the expected, mov_read_extradata will
 return 0 even without any truncation.
 In this case the error message would be incorrect

So should I code the test against codec id against the files I have or
the ones that might be valid, but I don't have samples for?

It looks like AV_CODEC_ID_AVUI and AV_CODEC_ID_DNXHD are tested for in
other parts of the code, so I'd be comfortable with those. However I'm
thinking the call to mov_read_avid might be skipped/inlined if I only
tested against the DNxHD codec, there is something that begins to look
like a code smell if I duplicate the list of codecs, but I'm happy to
go with whatever is preferred.

Thanks

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

[FFmpeg-devel] [PATCH] libavformat: DNxHD in .mov, switch unspecified color_range to mpeg

[Kevin Wheatley]

From 533523210ae02b80d4450793fd9e3865e716e858 Mon Sep 17 00:00:00 2001
From: Kevin Wheatley kevin.j.wheat...@gmail.com
Date: Tue, 10 Feb 2015 11:37:00 +
Subject: [PATCH] libavformat: DNxHD in .mov, switch unspecified color_range to mpeg

Avid prefers mpeg range [16-235] by default this change brings
ffmpeg into line with that. To obtain the old behaviour use
'-color_range jpeg' on the command line prior to the ouput
filename.

Signed-off-by: Kevin Wheatley kevin.j.wheat...@gmail.com
---
 Changelog |1 +
 libavformat/movenc.c  |3 ++-
 tests/ref/vsynth/vsynth1-dnxhd-1080i  |2 +-
 tests/ref/vsynth/vsynth1-dnxhd-1080i-colr |2 +-
 tests/ref/vsynth/vsynth2-dnxhd-1080i  |2 +-
 tests/ref/vsynth/vsynth2-dnxhd-1080i-colr |2 +-
 tests/ref/vsynth/vsynth3-dnxhd-1080i-colr |2 +-
 tests/ref/vsynth/vsynth_lena-dnxhd-1080i  |2 +-
 tests/ref/vsynth/vsynth_lena-dnxhd-1080i-colr |2 +-
 9 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/Changelog b/Changelog
index 8e55969..ca5e089 100644
--- a/Changelog
+++ b/Changelog
@@ -19,6 +19,7 @@ version next:
 - Twofish symmetric block cipher
 - Support DNx100 (960x720@8)
 - removed libmpcodecs
+- Changed default DNxHD colour range in QuickTime .mov derivatives to mpeg range
 
 
 version 2.5:
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index df70d57..f95d066 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1037,7 +1037,8 @@ static int mov_write_avid_tag(AVIOContext *pb, MOVTrack *track)
 ffio_wfourcc(pb, ACLR);
 ffio_wfourcc(pb, ACLR);
 ffio_wfourcc(pb, 0001);
-if (track-enc-color_range == AVCOL_RANGE_MPEG) { /* Legal range (16-235) */
+if (track-enc-color_range == AVCOL_RANGE_MPEG || /* Legal range (16-235) */
+track-enc-color_range == AVCOL_RANGE_UNSPECIFIED) {
 avio_wb32(pb, 1); /* Corresponds to 709 in official encoder */
 } else { /* Full range (0-255) */
 avio_wb32(pb, 2); /* Corresponds to RGB in official encoder */
diff --git a/tests/ref/vsynth/vsynth1-dnxhd-1080i b/tests/ref/vsynth/vsynth1-dnxhd-1080i
index f9b7e0e..28d55b6 100644
--- a/tests/ref/vsynth/vsynth1-dnxhd-1080i
+++ b/tests/ref/vsynth/vsynth1-dnxhd-1080i
@@ -1,4 +1,4 @@
-14f1ea20bbd3024ccbfd84c681888d07 *tests/data/fate/vsynth1-dnxhd-1080i.mov
+a0234e0a8516d958f423b119aa9e35c4 *tests/data/fate/vsynth1-dnxhd-1080i.mov
 3031911 tests/data/fate/vsynth1-dnxhd-1080i.mov
 a09132c6db44f415e831dcaa630a351b *tests/data/fate/vsynth1-dnxhd-1080i.out.rawvideo
 stddev:6.29 PSNR: 32.15 MAXDIFF:   64 bytes:  7603200/   760320
diff --git a/tests/ref/vsynth/vsynth1-dnxhd-1080i-colr b/tests/ref/vsynth/vsynth1-dnxhd-1080i-colr
index 5971f33..ee21e96 100644
--- a/tests/ref/vsynth/vsynth1-dnxhd-1080i-colr
+++ b/tests/ref/vsynth/vsynth1-dnxhd-1080i-colr
@@ -1,4 +1,4 @@
-b6fbfdfe7027fde6853930abad87eaab *tests/data/fate/vsynth1-dnxhd-1080i-colr.mov
+200a881696cc70c36c33fb415ed9488b *tests/data/fate/vsynth1-dnxhd-1080i-colr.mov
 3031929 tests/data/fate/vsynth1-dnxhd-1080i-colr.mov
 5835dff88cb84e83bbe70b5ed5edd5ab *tests/data/fate/vsynth1-dnxhd-1080i-colr.out.rawvideo
 stddev:5.79 PSNR: 32.87 MAXDIFF:   56 bytes:  7603200/   760320
diff --git a/tests/ref/vsynth/vsynth2-dnxhd-1080i b/tests/ref/vsynth/vsynth2-dnxhd-1080i
index 527e6a7..c3a5073 100644
--- a/tests/ref/vsynth/vsynth2-dnxhd-1080i
+++ b/tests/ref/vsynth/vsynth2-dnxhd-1080i
@@ -1,4 +1,4 @@
-d59d5d40ffb33ebb9e9e0d5025aefb88 *tests/data/fate/vsynth2-dnxhd-1080i.mov
+2b75889122f8d918e1b068d128b618ca *tests/data/fate/vsynth2-dnxhd-1080i.mov
 3031911 tests/data/fate/vsynth2-dnxhd-1080i.mov
 099001db73036eeb9545c463cf90f0ba *tests/data/fate/vsynth2-dnxhd-1080i.out.rawvideo
 stddev:1.53 PSNR: 44.43 MAXDIFF:   31 bytes:  7603200/   760320
diff --git a/tests/ref/vsynth/vsynth2-dnxhd-1080i-colr b/tests/ref/vsynth/vsynth2-dnxhd-1080i-colr
index 0994820..d41b5b3 100644
--- a/tests/ref/vsynth/vsynth2-dnxhd-1080i-colr
+++ b/tests/ref/vsynth/vsynth2-dnxhd-1080i-colr
@@ -1,4 +1,4 @@
-d510bc0d58c7cae875e3e67023771d6f *tests/data/fate/vsynth2-dnxhd-1080i-colr.mov
+53dfdc17882f2240a10c799287bf4b68 *tests/data/fate/vsynth2-dnxhd-1080i-colr.mov
 3031929 tests/data/fate/vsynth2-dnxhd-1080i-colr.mov
 e4cf5528c993b5e7d57a9d0a4d2cd0c6 *tests/data/fate/vsynth2-dnxhd-1080i-colr.out.rawvideo
 stddev:1.58 PSNR: 44.15 MAXDIFF:   33 bytes:  7603200/   760320
diff --git a/tests/ref/vsynth/vsynth3-dnxhd-1080i-colr b/tests/ref/vsynth/vsynth3-dnxhd-1080i-colr
index 926a7b2..2c5084c 100644
--- a/tests/ref/vsynth/vsynth3-dnxhd-1080i-colr
+++ b/tests/ref/vsynth/vsynth3-dnxhd-1080i-colr
@@ -1,4 +1,4 @@
-3b06d8675f9623db77b6a42916663608 *tests/data/fate/vsynth3-dnxhd-1080i-colr.mov
+da84414ce38ed0479c61a493398c912a *tests/data/fate/vsynth3-dnxhd-1080i-colr.mov
 3031929 tests/data/fate/vsynth3-dnxhd-1080i-colr.mov
 7dd6b261e439cda21df4f01b45336b41 *tests/data/fate/vsynth3-dnxhd-1080i

[FFmpeg-devel] avformat: QuickTime Sample Description Table 'corruption' with ACLR atom.

[Kevin Wheatley]

Hi,

currently when writing ACLR atoms to .mov's there is a 'corruption'
caused by the function mov_write_avid_tag() writing an additional 4
bytes of zero's. this is potentially then followed by other atoms colr
and pasp. Looking at the specifications it appears this 4 bytes is
supposed to occur at the end of the stsd if at all, as it is optional.

I'm working on a patch to fix the 'corruption' but wondered how to
handle the optional terminator:

1) Don't write one ever
2) Write one if requested by adding a flag to -movflags stdsterminator
3) Write one by default, add a 'nostdsterminator' flag (needs a better name)
4) always write one.

I'm favouring 2 or 3

Currently I've done option 1 see
https://github.com/KevinJW/FFmpeg/compare/mov_dnxhd_stsd_corruption

Thanks

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

Re: [FFmpeg-devel] avformat: QuickTime Sample Description Table 'corruption' with ACLR atom.

[Kevin Wheatley]

OK I'll proceed with that.

Thanks

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

Re: [FFmpeg-devel] avformat: QuickTime Sample Description Table 'corruption' with ACLR atom.

[Kevin Wheatley]

On Fri, Feb 13, 2015 at 3:06 PM, Michael Niedermayer michae...@gmx.at wrote:
 On Fri, Feb 13, 2015 at 02:43:13PM +, Kevin Wheatley wrote:

 what difference does the terminator make ?
 does it improve compatibility with other software ?


I believe there are some versions of Apple's Final Cut that need the
terminator, I've also been wondering if mp4 has the same needs but I
don't have the spec to cross check.

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

Re: [FFmpeg-devel] avformat: QuickTime Sample Description Table 'corruption' with ACLR atom.

[Kevin Wheatley]

I've rebased the patch against current master, it is a lot of FATE
changes due to the extra 4 bytes...

It should now merge appropriately, see https://github.com/FFmpeg/FFmpeg/pull/110

Please let me know if anything needs tweaking

Thanks

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

[FFmpeg-devel] [PATCH] avformat: Outputting DNxHD into .mov containers 'corrupts' following atoms until end of stsd

[Kevin Wheatley]

ffmpeg and qtdump could not decode pasp/colr atoms in the files made by ffmpeg,
when outputting DNxHD due to the incorrect padding placement. Now we add the
padding in the correct place, we also always add padding for Final Cut
compatibility.

Tidy up FATE changes due to padding changes.

Kevin
From b67ca5347a26227621054c82a688cc0ca44c11a1 Mon Sep 17 00:00:00 2001
From: Kevin Wheatley kevin.j.wheat...@gmail.com
Date: Mon, 16 Feb 2015 10:40:36 +
Subject: [PATCH] Outputting DNxHD into .mov containers 'corrupts' following atoms until end of stsd

ffmpeg and qtdump could not decode pasp/colr atoms in the files made by ffmpeg,
when outputting DNxHD due to the incorrect padding placement. Now we add the
padding in the correct place, we also always add padding for Final Cut
compatibility.

Tidy up FATE changes due to padding changes.

Signed-off-by: Kevin Wheatley kevin.j.wheat...@gmail.com
---
 Changelog |3 ++
 libavformat/movenc.c  |7 +++-
 tests/ref/lavf/ismv   |   12 +++---
 tests/ref/lavf/mov|   16 
 tests/ref/seek/lavf-mov   |   44 
 tests/ref/vsynth/vsynth1-avui |4 +-
 tests/ref/vsynth/vsynth1-dnxhd-1080i-colr |2 +-
 tests/ref/vsynth/vsynth1-mpeg4|4 +-
 tests/ref/vsynth/vsynth1-prores   |4 +-
 tests/ref/vsynth/vsynth1-prores_ks|4 +-
 tests/ref/vsynth/vsynth1-qtrle|4 +-
 tests/ref/vsynth/vsynth1-qtrlegray|4 +-
 tests/ref/vsynth/vsynth1-svq1 |4 +-
 tests/ref/vsynth/vsynth2-avui |4 +-
 tests/ref/vsynth/vsynth2-dnxhd-1080i-colr |2 +-
 tests/ref/vsynth/vsynth2-mpeg4|4 +-
 tests/ref/vsynth/vsynth2-prores   |4 +-
 tests/ref/vsynth/vsynth2-prores_ks|4 +-
 tests/ref/vsynth/vsynth2-qtrle|4 +-
 tests/ref/vsynth/vsynth2-qtrlegray|4 +-
 tests/ref/vsynth/vsynth2-svq1 |4 +-
 tests/ref/vsynth/vsynth3-dnxhd-1080i-colr |2 +-
 tests/ref/vsynth/vsynth3-mpeg4|4 +-
 tests/ref/vsynth/vsynth3-prores   |4 +-
 tests/ref/vsynth/vsynth3-prores_ks|4 +-
 tests/ref/vsynth/vsynth3-qtrle|4 +-
 tests/ref/vsynth/vsynth3-svq1 |4 +-
 tests/ref/vsynth/vsynth_lena-avui |4 +-
 tests/ref/vsynth/vsynth_lena-dnxhd-1080i-colr |2 +-
 tests/ref/vsynth/vsynth_lena-mpeg4|4 +-
 tests/ref/vsynth/vsynth_lena-prores   |4 +-
 tests/ref/vsynth/vsynth_lena-prores_ks|4 +-
 tests/ref/vsynth/vsynth_lena-qtrle|4 +-
 tests/ref/vsynth/vsynth_lena-qtrlegray|4 +-
 tests/ref/vsynth/vsynth_lena-svq1 |4 +-
 35 files changed, 100 insertions(+), 94 deletions(-)

diff --git a/Changelog b/Changelog
index 0bf7018..8080286 100644
--- a/Changelog
+++ b/Changelog
@@ -30,6 +30,9 @@ version next:
 - DV RTP payload format (RFC 6469) depacketizer
 - DXVA2-accelerated HEVC decoding
 - AAC ELD 480 decoding
+- Fix stsd atom corruption in DNxHD QuickTimes
+- Add 4 bytes stsd zero padding to video track for all QuickTime outputs
+
 
 version 2.5:
 - HEVC/H.265 RTP payload format (draft v6) packetizer
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index f95d066..e8b3a6b 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1077,8 +1077,6 @@ static int mov_write_avid_tag(AVIOContext *pb, MOVTrack *track)
 for (i = 0; i  10; i++)
 avio_wb64(pb, 0);
 
-/* extra padding for stsd needed */
-avio_wb32(pb, 0);
 return 0;
 }
 
@@ -1674,6 +1672,11 @@ static int mov_write_video_tag(AVIOContext *pb, MOVMuxContext *mov, MOVTrack *tr
 mov_write_pasp_tag(pb, track);
 }
 
+/* extra padding for stsd needed */
+/* https://developer.apple.com/library/mac/documentation/QuickTime/QTFF/QTFFChap2/qtff2.html#//apple_ref/doc/uid/TP4939-CH204-61112 */
+/* suggests it is optional, but there are suggestions that Final Cut may need it. */
+avio_wb32(pb, 0);
+
 return update_size(pb, pos);
 }
 
diff --git a/tests/ref/lavf/ismv b/tests/ref/lavf/ismv
index f29b5ff..ea8ea3a 100644
--- a/tests/ref/lavf/ismv
+++ b/tests/ref/lavf/ismv
@@ -1,9 +1,9 @@
-a9ccbb4cd1436d222ef4425567b4e03d *./tests/data/lavf/lavf.ismv
-312542 ./tests/data/lavf/lavf.ismv
+e26cf1bc88f31010c4c34af8eb9580ec *./tests/data/lavf/lavf.ismv
+312546 ./tests/data/lavf/lavf.ismv
 ./tests/data/lavf/lavf.ismv CRC=0x9d9a638a
-440d85f9fd5b9f63c2676638782b5c15 *./tests/data/lavf/lavf.ismv
-321448 ./tests/data/lavf/lavf.ismv
+ed0124f2dab1204869f3afed20d631c9 *./tests/data/lavf/lavf.ismv
+321452 ./tests/data/lavf/lavf.ismv
 ./tests/data/lavf/lavf.ismv CRC=0xe8130120
-a9ccbb4cd1436d222ef4425567b4e03d *./tests/data/lavf/lavf.ismv
-312542 ./tests

[FFmpeg-devel] [PATCH] optionally write QuickTime gama atom (was Re: Gamma function)

[Kevin Wheatley]

Updated to use lrint()

On Mon, Mar 2, 2015 at 12:54 PM, Kevin Wheatley
kevin.j.wheat...@gmail.com wrote:
 Something like this - note this adds no tests, but fate still passes for me.

 Kevin
From 794c8c3cf1e8506393fbcb4ddf1360042b0fc82f Mon Sep 17 00:00:00 2001
From: Kevin Wheatley kevin.j.wheat...@gmail.com
Date: Mon, 2 Mar 2015 12:50:53 +
Subject: [PATCH] Add 'gama' atom for .mov only, reuses pngenc.c gamma values by sharing
 function in new color_utils.[ch] file.

Use lrint() instead of round()

Signed-off-by: Kevin Wheatley kevin.j.wheat...@gmail.com
---
 libavcodec/pngenc.c |   29 +++--
 libavformat/movenc.c|   35 +++
 libavformat/movenc.h|2 +
 libavutil/Makefile  |2 +
 libavutil/color_utils.c |   52 +++
 libavutil/color_utils.h |   29 ++
 6 files changed, 124 insertions(+), 25 deletions(-)
 create mode 100644 libavutil/color_utils.c
 create mode 100644 libavutil/color_utils.h

diff --git a/libavcodec/pngenc.c b/libavcodec/pngenc.c
index 4e67ce2..fddcca4 100644
--- a/libavcodec/pngenc.c
+++ b/libavcodec/pngenc.c
@@ -28,6 +28,7 @@
 #include libavutil/avassert.h
 #include libavutil/libm.h
 #include libavutil/opt.h
+#include libavutil/color_utils.h
 
 #include zlib.h
 
@@ -277,31 +278,9 @@ static int png_get_chrm(enum AVColorPrimaries prim,  uint8_t *buf)
 
 static int png_get_gama(enum AVColorTransferCharacteristic trc, uint8_t *buf)
 {
-double gamma;
-switch (trc) {
-case AVCOL_TRC_BT709:
-case AVCOL_TRC_SMPTE170M:
-case AVCOL_TRC_SMPTE240M:
-case AVCOL_TRC_BT1361_ECG:
-case AVCOL_TRC_BT2020_10:
-case AVCOL_TRC_BT2020_12:
-/* these share a segmented TRC, but gamma 1.961 is a close
-  approximation, and also more correct for decoding content */
-gamma = 1.961;
-break;
-case AVCOL_TRC_GAMMA22:
-case AVCOL_TRC_IEC61966_2_1:
-gamma = 2.2;
-break;
-case AVCOL_TRC_GAMMA28:
-gamma = 2.8;
-break;
-case AVCOL_TRC_LINEAR:
-gamma = 1.0;
-break;
-default:
-return 0;
-}
+double gamma = get_gamma_from_trc(trc);
+if (gamma = 1e-6)
+return 0;
 
 AV_WB32_PNG(buf, 1.0 / gamma);
 return 1;
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index e496ba4..7c10d0e 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -40,10 +40,12 @@
 #include libavutil/avstring.h
 #include libavutil/intfloat.h
 #include libavutil/mathematics.h
+#include libavutil/libm.h
 #include libavutil/opt.h
 #include libavutil/dict.h
 #include libavutil/pixdesc.h
 #include libavutil/timecode.h
+#include libavutil/color_utils.h
 #include hevc.h
 #include rtpenc.h
 #include mov_chan.h
@@ -65,6 +67,7 @@ static const AVOption options[] = {
 { frag_discont, Signal that the next fragment is discontinuous from earlier ones, 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_FRAG_DISCONT}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, movflags },
 { delay_moov, Delay writing the initial moov until the first fragment is cut, or until the first fragment flush, 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_DELAY_MOOV}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, movflags },
 { write_colr, Write colr atom, 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_WRITE_COLR}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, movflags },
+{ write_gama, Write deprecated gama atom, 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_WRITE_GAMA}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, movflags },
 FF_RTP_FLAG_OPTS(MOVMuxContext, rtp_flags),
 { skip_iods, Skip writing iods atom., offsetof(MOVMuxContext, iods_skip), AV_OPT_TYPE_INT, {.i64 = 1}, 0, 1, AV_OPT_FLAG_ENCODING_PARAM},
 { iods_audio_profile, iods audio profile atom., offsetof(MOVMuxContext, iods_audio_profile), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 255, AV_OPT_FLAG_ENCODING_PARAM},
@@ -77,6 +80,7 @@ static const AVOption options[] = {
 { brand,Override major brand, offsetof(MOVMuxContext, major_brand),   AV_OPT_TYPE_STRING, {.str = NULL}, .flags = AV_OPT_FLAG_ENCODING_PARAM },
 { use_editlist, use edit list, offsetof(MOVMuxContext, use_editlist), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 1, AV_OPT_FLAG_ENCODING_PARAM},
 { fragment_index, Fragment number of the next fragment, offsetof(MOVMuxContext, fragments), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
+{ mov_gamma, gamma value for gama atom, offsetof(MOVMuxContext, gamma), AV_OPT_TYPE_FLOAT, {.dbl = 0.0 }, 0.0, 10, AV_OPT_FLAG_ENCODING_PARAM},
 { NULL },
 };
 
@@ -1519,6 +1523,31 @@ static int mov_write_pasp_tag(AVIOContext *pb, MOVTrack *track)
 return 16;
 }
 
+static int mov_write_gama_tag(AVIOContext *pb, MOVTrack *track, double gamma)
+{
+uint32_t gama = 0;
+if (gamma = 0.0)
+{
+gamma

Re: [FFmpeg-devel] Gamma function (was Re: [PATCH] lavc/pngenc: Support writing colorspace tags.)

[Kevin Wheatley]

On Mon, Mar 2, 2015 at 12:17 PM, Michael Niedermayer michae...@gmx.at wrote:
 the various colorspace options should pass from decoder over the
 video filter chain to the encoder and then muxer
 its best to change them from the command line through a video
 filter, this also ensures that the value reaching the encoder and
 muxer matches


I think I understand what you are suggesting in general, but in this
case this is an old metadata option that I don't imagine any other
format would want to propagate, it existed prior to the colr/nclc/nclx
and is  mostly superceded by that. I say mostly because the trc
options don't allow for arbitrary values like '2.6' for instance, and
we've also come across apps that need the gama atom setting to a value
and the trc left as 'unspecified' in order to correctly decode. even
when rec709 would have been the correct value.

That was why I was thinking it should be a .mov muxer only option.

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

Re: [FFmpeg-devel] Gamma function (was Re: [PATCH] lavc/pngenc: Support writing colorspace tags.)

[Kevin Wheatley]

Something like this - note this adds no tests, but fate still passes for me.

Kevin
From db02ae26c3c4278da4ed328e767bab98271da51e Mon Sep 17 00:00:00 2001
From: Kevin Wheatley kevin.j.wheat...@gmail.com
Date: Mon, 2 Mar 2015 12:50:53 +
Subject: [PATCH] Add 'gama' atom for .mov only, reuses pngenc.c gamma values by sharing
 function in new color_utils.[ch] file.


Signed-off-by: Kevin Wheatley kevin.j.wheat...@gmail.com
---
 libavcodec/pngenc.c |   29 +++--
 libavformat/movenc.c|   34 ++
 libavformat/movenc.h|2 +
 libavutil/Makefile  |2 +
 libavutil/color_utils.c |   52 +++
 libavutil/color_utils.h |   29 ++
 6 files changed, 123 insertions(+), 25 deletions(-)
 create mode 100644 libavutil/color_utils.c
 create mode 100644 libavutil/color_utils.h

diff --git a/libavcodec/pngenc.c b/libavcodec/pngenc.c
index 9fd8eef..db849fb 100644
--- a/libavcodec/pngenc.c
+++ b/libavcodec/pngenc.c
@@ -27,6 +27,7 @@
 
 #include libavutil/avassert.h
 #include libavutil/opt.h
+#include libavutil/color_utils.h
 
 #include zlib.h
 
@@ -276,31 +277,9 @@ static int png_get_chrm(enum AVColorPrimaries prim,  uint8_t *buf)
 
 static int png_get_gama(enum AVColorTransferCharacteristic trc, uint8_t *buf)
 {
-double gamma;
-switch (trc) {
-case AVCOL_TRC_BT709:
-case AVCOL_TRC_SMPTE170M:
-case AVCOL_TRC_SMPTE240M:
-case AVCOL_TRC_BT1361_ECG:
-case AVCOL_TRC_BT2020_10:
-case AVCOL_TRC_BT2020_12:
-/* these share a segmented TRC, but gamma 1.961 is a close
-  approximation, and also more correct for decoding content */
-gamma = 1.961;
-break;
-case AVCOL_TRC_GAMMA22:
-case AVCOL_TRC_IEC61966_2_1:
-gamma = 2.2;
-break;
-case AVCOL_TRC_GAMMA28:
-gamma = 2.8;
-break;
-case AVCOL_TRC_LINEAR:
-gamma = 1.0;
-break;
-default:
-return 0;
-}
+double gamma = get_gamma_from_trc(trc);
+if (gamma = 1e-6)
+return 0;
 
 AV_WB32_PNG(buf, 1.0 / gamma);
 return 1;
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index e496ba4..05516a8 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -44,6 +44,7 @@
 #include libavutil/dict.h
 #include libavutil/pixdesc.h
 #include libavutil/timecode.h
+#include libavutil/color_utils.h
 #include hevc.h
 #include rtpenc.h
 #include mov_chan.h
@@ -65,6 +66,7 @@ static const AVOption options[] = {
 { frag_discont, Signal that the next fragment is discontinuous from earlier ones, 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_FRAG_DISCONT}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, movflags },
 { delay_moov, Delay writing the initial moov until the first fragment is cut, or until the first fragment flush, 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_DELAY_MOOV}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, movflags },
 { write_colr, Write colr atom, 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_WRITE_COLR}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, movflags },
+{ write_gama, Write deprecated gama atom, 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_WRITE_GAMA}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, movflags },
 FF_RTP_FLAG_OPTS(MOVMuxContext, rtp_flags),
 { skip_iods, Skip writing iods atom., offsetof(MOVMuxContext, iods_skip), AV_OPT_TYPE_INT, {.i64 = 1}, 0, 1, AV_OPT_FLAG_ENCODING_PARAM},
 { iods_audio_profile, iods audio profile atom., offsetof(MOVMuxContext, iods_audio_profile), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 255, AV_OPT_FLAG_ENCODING_PARAM},
@@ -77,6 +79,7 @@ static const AVOption options[] = {
 { brand,Override major brand, offsetof(MOVMuxContext, major_brand),   AV_OPT_TYPE_STRING, {.str = NULL}, .flags = AV_OPT_FLAG_ENCODING_PARAM },
 { use_editlist, use edit list, offsetof(MOVMuxContext, use_editlist), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 1, AV_OPT_FLAG_ENCODING_PARAM},
 { fragment_index, Fragment number of the next fragment, offsetof(MOVMuxContext, fragments), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
+{ mov_gamma, gamma value for gama atom, offsetof(MOVMuxContext, gamma), AV_OPT_TYPE_FLOAT, {.dbl = 0.0 }, 0.0, 10, AV_OPT_FLAG_ENCODING_PARAM},
 { NULL },
 };
 
@@ -1519,6 +1522,31 @@ static int mov_write_pasp_tag(AVIOContext *pb, MOVTrack *track)
 return 16;
 }
 
+static int mov_write_gama_tag(AVIOContext *pb, MOVTrack *track, double gamma)
+{
+uint32_t gama = 0;
+if (gamma = 0.0)
+{
+gamma = get_gamma_from_trc(track-enc-color_trc);
+}
+av_log(pb, AV_LOG_DEBUG, gamma value %g\n, gamma);
+
+if (gamma  1e-6) {
+gama = (uint32_t)round((double)(116) * gamma);
+av_log(pb, AV_LOG_DEBUG, writing gama value %d\n, gama);
+
+av_assert0(track-mode == MODE_MOV

[FFmpeg-devel] Gamma function (was Re: [PATCH] lavc/pngenc: Support writing colorspace tags.)

[Kevin Wheatley]

On Sat, Feb 28, 2015 at 1:50 AM, Niklas Haas g...@nand.wakku.to wrote:

 +static int png_get_gama(enum AVColorTransferCharacteristic trc, uint8_t *buf)
 +{
 +double gamma;
 +switch (trc) {
 +case AVCOL_TRC_BT709:
 +case AVCOL_TRC_SMPTE170M:
 +case AVCOL_TRC_SMPTE240M:
 +case AVCOL_TRC_BT1361_ECG:
 +case AVCOL_TRC_BT2020_10:
 +case AVCOL_TRC_BT2020_12:
 +/* these share a segmented TRC, but gamma 1.961 is a close
 +  approximation, and also more correct for decoding content */
 +gamma = 1.961;
 +break;
 +case AVCOL_TRC_GAMMA22:
 +case AVCOL_TRC_IEC61966_2_1:
 +gamma = 2.2;
 +break;
 +case AVCOL_TRC_GAMMA28:
 +gamma = 2.8;
 +break;
 +case AVCOL_TRC_LINEAR:
 +gamma = 1.0;
 +break;
 +default:
 +return 0;
 +}
 +
 +AV_WB32_PNG(buf, 1.0 / gamma);
 +return 1;
 +}
 +


Co-incidentally I was thinking of submitting a patch to write the old
QuickTime gama atom and most of this function would be useful if
extracted to a common place (everything but the AV_WB32_PNG() as
.mov's need a different scaling). Where should it go? libavutil?

As part of it I would also allow manually specifying a specific value
to encode on the command line, but I wondered about how to form the
command line options...

Case 1

User wants to write gamma based on TRC settings, using the above (now
extracted) function.

+write_gama on movflags?

Case 2

User wants to specify the actual gamma on the command line

Same as above, plus a  -gamma double

I'd suggest to require +write_gama on the command line as the gama
atom is deprecated and should really be only needed for backwards
compatibity/broken tools.

Comments?

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

Re: [FFmpeg-devel] [PATCH] optionally write QuickTime gama atom (was Re: Gamma function)

[Kevin Wheatley]

Michael, wm4,

Thank you for your feedback, I believe I've addressed your concerns,
please let me know if there is anything else that needs fixing.

Kevin
From 079ff77a1885b5ef879a8cd3b4c032a3182e8e67 Mon Sep 17 00:00:00 2001
From: Kevin Wheatley kevin.j.wheat...@gmail.com
Date: Thu, 5 Mar 2015 10:37:51 +
Subject: [PATCH 1/2] Extract gamma determination from PNG encoder for future use.
 Adds private avpriv_get_gamma_from_trc() function to libavutil.


Signed-off-by: Kevin Wheatley kevin.j.wheat...@gmail.com
---
 libavcodec/pngenc.c |   29 +++--
 libavutil/Makefile  |1 +
 libavutil/color_utils.c |   52 +++
 libavutil/color_utils.h |   39 +++
 libavutil/version.h |2 +-
 5 files changed, 97 insertions(+), 26 deletions(-)
 create mode 100644 libavutil/color_utils.c
 create mode 100644 libavutil/color_utils.h

diff --git a/libavcodec/pngenc.c b/libavcodec/pngenc.c
index 6c9f43e..9bdefc4 100644
--- a/libavcodec/pngenc.c
+++ b/libavcodec/pngenc.c
@@ -28,6 +28,7 @@
 #include libavutil/avassert.h
 #include libavutil/libm.h
 #include libavutil/opt.h
+#include libavutil/color_utils.h
 
 #include zlib.h
 
@@ -277,31 +278,9 @@ static int png_get_chrm(enum AVColorPrimaries prim,  uint8_t *buf)
 
 static int png_get_gama(enum AVColorTransferCharacteristic trc, uint8_t *buf)
 {
-double gamma;
-switch (trc) {
-case AVCOL_TRC_BT709:
-case AVCOL_TRC_SMPTE170M:
-case AVCOL_TRC_SMPTE240M:
-case AVCOL_TRC_BT1361_ECG:
-case AVCOL_TRC_BT2020_10:
-case AVCOL_TRC_BT2020_12:
-/* these share a segmented TRC, but gamma 1.961 is a close
-  approximation, and also more correct for decoding content */
-gamma = 1.961;
-break;
-case AVCOL_TRC_GAMMA22:
-case AVCOL_TRC_IEC61966_2_1:
-gamma = 2.2;
-break;
-case AVCOL_TRC_GAMMA28:
-gamma = 2.8;
-break;
-case AVCOL_TRC_LINEAR:
-gamma = 1.0;
-break;
-default:
-return 0;
-}
+double gamma = avpriv_get_gamma_from_trc(trc);
+if (gamma = 1e-6)
+return 0;
 
 AV_WB32_PNG(buf, 1.0 / gamma);
 return 1;
diff --git a/libavutil/Makefile b/libavutil/Makefile
index 6caf896..df85cd1 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -88,6 +88,7 @@ OBJS = adler32.o\
cast5.o  \
camellia.o   \
channel_layout.o \
+   color_utils.o\
cpu.o\
crc.o\
des.o\
diff --git a/libavutil/color_utils.c b/libavutil/color_utils.c
new file mode 100644
index 000..59146be
--- /dev/null
+++ b/libavutil/color_utils.c
@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2015 Kevin Wheatley kevin.j.wheat...@gmail.com
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include libavutil/color_utils.h
+#include libavutil/pixfmt.h
+
+double avpriv_get_gamma_from_trc(enum AVColorTransferCharacteristic trc)
+{
+double gamma;
+switch (trc) {
+case AVCOL_TRC_BT709:
+case AVCOL_TRC_SMPTE170M:
+case AVCOL_TRC_SMPTE240M:
+case AVCOL_TRC_BT1361_ECG:
+case AVCOL_TRC_BT2020_10:
+case AVCOL_TRC_BT2020_12:
+/* these share a segmented TRC, but gamma 1.961 is a close
+  approximation, and also more correct for decoding content */
+gamma = 1.961;
+break;
+case AVCOL_TRC_GAMMA22:
+case AVCOL_TRC_IEC61966_2_1:
+gamma = 2.2;
+break;
+case AVCOL_TRC_GAMMA28:
+gamma = 2.8;
+break;
+case AVCOL_TRC_LINEAR:
+gamma = 1.0;
+break;
+default:
+gamma = 0.0; // Unknown

Re: [FFmpeg-devel] [PATCH] avformat: Outputting DNxHD into .mov containers 'corrupts' following atoms until end of stsd

[Kevin Wheatley]

On Fri, Feb 20, 2015 at 11:36 AM, Michael Niedermayer michae...@gmx.at wrote:
 applied the case for DNxHD, for the more general case, please
 explain which case(s) and software need them, and how to reproduce
 that

My experience and by the looks of things other people using
libquicktime have seen issues with Final Cut having problems reading
the files if the stds

http://libquicktime.cvs.sourceforge.net/viewvc/libquicktime/libquicktime/src/stsdtable.c?view=markup

quicktime_write_stsd_video() line 643 is where they sometimes pad.

http://comments.gmane.org/gmane.comp.video.libquicktime.devel/1348

appears to be the discussion around why they do it

 I dont see where the text would allow one to add such padding by
 ones own choice. I just see a note that says that parsers need to
 cope with it. Thats not the same as saying its ok to add it in all
 cases.
 But maybe iam missing something, i didnt read the whole document

no I would agree it only indicates it is optional

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

Re: [FFmpeg-devel] Gaining access to a encoder context in avformat?

[Kevin Wheatley]

On Fri, Feb 20, 2015 at 5:51 PM, Michael Niedermayer michae...@gmx.at wrote:

 theres some code that memcpies extradata into vos_data and that is
 skiped if TAG_IS_AVCI(trk-tag), try to also skip this for DNxHD

Michael,

thanks for the pointer, there are actually two points in the code that
appear to need the guard against overwriting, I'm attaching an updated
patch for comments.

Kevin
From bd5543cd6a227e64e66eb5ac909e5efeddfeb3a8 Mon Sep 17 00:00:00 2001
From: Kevin Wheatley kevin.j.wheat...@gmail.com
Date: Tue, 24 Feb 2015 10:00:07 +
Subject: [PATCH] Using the copy codec ACLR atoms are incorrectly written - fix.

During the creation of the ACLR atom we are assuming the vos_data
contains the DNxHD header. This change makes this explicit and
ensures we don't over write the stream with the extra_data.

Signed-off-by: Kevin Wheatley kevin.j.wheat...@gmail.com
---
 libavformat/movenc.c |   31 +++
 1 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 210f78e..276b711 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1033,6 +1033,27 @@ static int mov_write_hvcc_tag(AVIOContext *pb, MOVTrack *track)
 static int mov_write_avid_tag(AVIOContext *pb, MOVTrack *track)
 {
 int i;
+int interlaced;
+int cid;
+
+if (track-vos_data  track-vos_len  0x29) { 
+if (track-vos_data[0] == 0x00 
+track-vos_data[1] == 0x00 
+track-vos_data[2] == 0x02 
+track-vos_data[3] == 0x80 
+(track-vos_data[4] == 0x01 || track-vos_data[4] == 0x02)) {
+/* looks like a DNxHD bit stream */
+interlaced = (track-vos_data[5]  2);
+cid = AV_RB32(track-vos_data + 0x28);
+} else {
+av_log(NULL, AV_LOG_WARNING, Could not locate DNxHD bit stream in vos_data\n);
+return 0;
+}
+} else {
+av_log(NULL, AV_LOG_WARNING, Could not locate DNxHD bit stream, vos_data too small\n);
+return 0;
+}
+
 avio_wb32(pb, 24); /* size */
 ffio_wfourcc(pb, ACLR);
 ffio_wfourcc(pb, ACLR);
@@ -1056,10 +1077,10 @@ static int mov_write_avid_tag(AVIOContext *pb, MOVTrack *track)
 ffio_wfourcc(pb, ARES);
 ffio_wfourcc(pb, ARES);
 ffio_wfourcc(pb, 0001);
-avio_wb32(pb, AV_RB32(track-vos_data + 0x28)); /* dnxhd cid, some id ? */
+avio_wb32(pb, cid); /* dnxhd cid, some id ? */
 avio_wb32(pb, track-enc-width);
 /* values below are based on samples created with quicktime and avid codecs */
-if (track-vos_data[5]  2) { // interlaced
+if (interlaced) {
 avio_wb32(pb, track-enc-height / 2);
 avio_wb32(pb, 2); /* unknown */
 avio_wb32(pb, 0); /* unknown */
@@ -4165,7 +4186,9 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
 samples_in_chunk = 1;
 
 /* copy extradata if it exists */
-if (trk-vos_len == 0  enc-extradata_size  0  !TAG_IS_AVCI(trk-tag)) {
+if (trk-vos_len == 0  enc-extradata_size  0 
+!TAG_IS_AVCI(trk-tag) 
+(enc-codec_id != AV_CODEC_ID_DNXHD)) {
 trk-vos_len  = enc-extradata_size;
 trk-vos_data = av_malloc(trk-vos_len);
 if (!trk-vos_data) {
@@ -4952,7 +4975,7 @@ static int mov_write_header(AVFormatContext *s)
 if (st-codec-extradata_size) {
 if (st-codec-codec_id == AV_CODEC_ID_DVD_SUBTITLE)
 mov_create_dvd_sub_decoder_specific_info(track, st);
-else if (!TAG_IS_AVCI(track-tag)){
+else if (!TAG_IS_AVCI(track-tag)  st-codec-codec_id != AV_CODEC_ID_DNXHD) {
 track-vos_len  = st-codec-extradata_size;
 track-vos_data = av_malloc(track-vos_len);
 if (!track-vos_data) {
-- 
1.7.1

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

Re: [FFmpeg-devel] [GSoC] Patch which adds support for gamma correct scaling

[Kevin Wheatley]

Pedro

I understand the desire to 'degamma' images prior to scaling, I'd be
interested in understanding how this could work with the colour_trc
options rather than only assuming a simple gamma.

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

Re: [FFmpeg-devel] [GSoC] Patch which adds support for gamma correct scaling

[Kevin Wheatley]

I'll add that for some encodings filters with negative lobes will
cause ringing no matter the direction up/down, especially true for
higher dynamic range encodings like log and HDR options like SMPTE ST
2084

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

Re: [FFmpeg-devel] [PATCH]Remove framerates from dnxhd encoder help

[Kevin Wheatley]

Though not definitive in terms of a specification:

http://www.avid.com/static/resources/us/documents/dnxhd.pdf

pages 9/10 has the table of options.

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

[FFmpeg-devel] Enhancing/extending OpenEXR 'gamma' feature

[Kevin Wheatley]

HI,

I was wondering what others think about the option of extending the
OpenEXR reader's gamma option to also include various transfer
characteristic curves, potentially corresponding to the various
AVCOL_TRC_* options, though I don't immediately have a need for all of
them.

In addition I could see the need for other curves like those of cinema
camera vendor log curves, for instance Red, Sony, ARRI.

From a user point of view I'd see a command line option specific to
the EXR codec that would take a subset of the colour_trc options
(bt709, etc) extended to include some camera transfer curves and it
would apply the appropriate function prior to the encoding as
AV_PIX_FMT_RGB48/64 as is done currently.

I'm thinking the command line option would be called 'apply_trc'  or
something similar.

Why it is needed is simply that the gamma function isn't really
sufficient for matching all the typical formats we see in production
use.

What would be nice would be to embed more colour adjustments prior to
the truncation to integer values but that could be quite complex and
may be better handled outside of ffmpeg.

I've seen that there was a mention of something similar in the list
archive, but could not find anything other than those emails.

I'd be great full if any of the devs could comment on the idea before
I start on the implementation.

Thanks

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

Re: [FFmpeg-devel] Enhancing/extending OpenEXR 'gamma' feature

[Kevin Wheatley]

On Fri, Aug 28, 2015 at 3:54 PM, Paul B Mahol one...@gmail.com wrote:
 On 8/28/15, Kevin Wheatley kevin.j.wheat...@gmail.com wrote:
 Feel free to do it.

 Note however that those various curves really belong to avfilter once
 swscale supports float pixel format and exr decoder make use of it.

Absolutely, most of the tools I normally work with are full
float/double internally, I was going to start with the functions that
perform the non-linear encoding, but if I'm to consider the option
they will need to work in the swscale code it opens a few questions:

Should I provide functions going too and from linear? This would allow
them to be composed to perform cross conversion.
float or double precision? - if these functions will be composed
together is there a need for double precision?

Thanks

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

[FFmpeg-devel] [PATCH] avutil: Add additional primaries and transfer characteristic enumerations from ITU-T Rec H.265

[Kevin Wheatley]

As a starting point for my work, I'm wondering about the naming of
these options in the following patch to add the additional transfer
characteristics from H.265.

I'm not entirely happy with the names as they read a little
ambiguously SMPTE ST vs SMP TEST, obviously I could add another
underscore...

Thoughts

Kevin
From 6190c0ee42fd2cb4ea1370111b46e03ea55459af Mon Sep 17 00:00:00 2001
From: Kevin Wheatley <kevin.j.wheat...@gmail.com>
Date: Fri, 28 Aug 2015 15:20:22 +0100
Subject: [PATCH] Add additional primaries and transfer characteristic enumerations from ITU-T Rec H.265


Signed-off-by: Kevin Wheatley <kevin.j.wheat...@gmail.com>
---
 libavutil/pixfmt.h |   21 -
 1 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
index 029c911..0f93050 100644
--- a/libavutil/pixfmt.h
+++ b/libavutil/pixfmt.h
@@ -475,17 +475,18 @@ enum AVPixelFormat {
   */
 enum AVColorPrimaries {
 AVCOL_PRI_RESERVED0   = 0,
-AVCOL_PRI_BT709   = 1, ///< also ITU-R BT1361 / IEC 61966-2-4 / SMPTE RP177 Annex B
+AVCOL_PRI_BT709   = 1,  ///< also ITU-R BT1361 / IEC 61966-2-4 / SMPTE RP177 Annex B
 AVCOL_PRI_UNSPECIFIED = 2,
 AVCOL_PRI_RESERVED= 3,
-AVCOL_PRI_BT470M  = 4, ///< also FCC Title 47 Code of Federal Regulations 73.682 (a)(20)
-
-AVCOL_PRI_BT470BG = 5, ///< also ITU-R BT601-6 625 / ITU-R BT1358 625 / ITU-R BT1700 625 PAL & SECAM
-AVCOL_PRI_SMPTE170M   = 6, ///< also ITU-R BT601-6 525 / ITU-R BT1358 525 / ITU-R BT1700 NTSC
-AVCOL_PRI_SMPTE240M   = 7, ///< functionally identical to above
-AVCOL_PRI_FILM= 8, ///< colour filters using Illuminant C
-AVCOL_PRI_BT2020  = 9, ///< ITU-R BT2020
-AVCOL_PRI_NB,  ///< Not part of ABI
+AVCOL_PRI_BT470M  = 4,  ///< also FCC Title 47 Code of Federal Regulations 73.682 (a)(20)
+
+AVCOL_PRI_BT470BG = 5,  ///< also ITU-R BT601-6 625 / ITU-R BT1358 625 / ITU-R BT1700 625 PAL & SECAM
+AVCOL_PRI_SMPTE170M   = 6,  ///< also ITU-R BT601-6 525 / ITU-R BT1358 525 / ITU-R BT1700 NTSC
+AVCOL_PRI_SMPTE240M   = 7,  ///< functionally identical to above
+AVCOL_PRI_FILM= 8,  ///< colour filters using Illuminant C
+AVCOL_PRI_BT2020  = 9,  ///< ITU-R BT2020
+AVCOL_PRI_SMPTE428_1  = 10, ///< SMPTE ST 428-1 (CIE 1931 XYZ)
+AVCOL_PRI_NB,   ///< Not part of ABI
 };
 
 /**
@@ -508,6 +509,8 @@ enum AVColorTransferCharacteristic {
 AVCOL_TRC_IEC61966_2_1 = 13, ///< IEC 61966-2-1 (sRGB or sYCC)
 AVCOL_TRC_BT2020_10= 14, ///< ITU-R BT2020 for 10 bit system
 AVCOL_TRC_BT2020_12= 15, ///< ITU-R BT2020 for 12 bit system
+AVCOL_TRC_SMPTEST2084  = 16, ///< SMPTE ST 2084 for 10, 12, 14 and 16 bit systems
+AVCOL_TRC_SMPTEST428_1 = 17, ///< SMPTE ST 428-1
 AVCOL_TRC_NB,///< Not part of ABI
 };
 
-- 
1.7.1

From b11126e0e80589d0ac6e12b301d846cd90b9caef Mon Sep 17 00:00:00 2001
From: Kevin Wheatley <kevin.j.wheat...@gmail.com>
Date: Tue, 1 Sep 2015 09:14:01 +0100
Subject: [PATCH 2/2] Add CLI options for SMPTE ST 2084 and ST 428-1 transfer characteristics


Signed-off-by: Kevin Wheatley <kevin.j.wheat...@gmail.com>
---
 libavcodec/options_table.h |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
index 93d4f77..f698262 100644
--- a/libavcodec/options_table.h
+++ b/libavcodec/options_table.h
@@ -444,6 +444,8 @@ static const AVOption avcodec_options[] = {
 {"iec61966_2_1", "IEC 61966-2-1",0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_TRC_IEC61966_2_1 }, INT_MIN, INT_MAX, V|E|D, "color_trc_type"},
 {"bt2020_10bit", "BT.2020 - 10 bit", 0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_TRC_BT2020_10 },INT_MIN, INT_MAX, V|E|D, "color_trc_type"},
 {"bt2020_12bit", "BT.2020 - 12 bit", 0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_TRC_BT2020_12 },INT_MIN, INT_MAX, V|E|D, "color_trc_type"},
+{"smpte2084","SMPTE ST 2084",0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_TRC_SMPTEST2084 },  INT_MIN, INT_MAX, V|E|D, "color_trc_type"},
+{"smpte428_1",   "SMPTE ST 428-1",   0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_TRC_SMPTEST428_1 }, INT_MIN, INT_MAX, V|E|D, "color_trc_type"},
 {"colorspace", "color space", OFFSET(colorspace), AV_OPT_TYPE_INT, {.i64 = AVCOL_SPC_UNSPECIFIED }, 0, AVCOL_SPC_NB-1, V|E|D, "colorspace_type"},
 {"rgb", "RGB", 0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_SPC_RGB }, INT_MIN, INT_MAX, V|E|D, "colorspace_type"},
 {"bt709",   "BT.709",  0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_SPC_BT709 },       INT_MIN, INT_MAX, V|E|D, "colorspace_type"},
-- 
1.7.1

Re: [FFmpeg-devel] [PATCH] avutil: Add additional primaries and transfer characteristic enumerations from ITU-T Rec H.265

[Kevin Wheatley]

I noticed that the output from actually tagging buffers with the new
states needs patching as well, is there any other place I need to
update?

Kevin

On Tue, Sep 1, 2015 at 9:39 AM, Kevin Wheatley
<kevin.j.wheat...@gmail.com> wrote:
> As a starting point for my work, I'm wondering about the naming of
> these options in the following patch to add the additional transfer
> characteristics from H.265.
>
> I'm not entirely happy with the names as they read a little
> ambiguously SMPTE ST vs SMP TEST, obviously I could add another
> underscore...
>
> Thoughts
>
> Kevin
From 403a209cfe8ebee7caca35b678f8d529e8ae5390 Mon Sep 17 00:00:00 2001
From: Kevin Wheatley <kevin.j.wheat...@gmail.com>
Date: Tue, 1 Sep 2015 12:34:34 +0100
Subject: [PATCH 6/7] Add SMPTE ST 2084 and ST 428-1 pixel descriptions

---
 libavutil/pixdesc.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
index eb52113..4ef5d83 100644
--- a/libavutil/pixdesc.c
+++ b/libavutil/pixdesc.c
@@ -2013,13 +2013,14 @@ static const char *color_range_names[AVCOL_RANGE_NB] = {
 static const char *color_primaries_names[AVCOL_PRI_NB] = {
 "reserved", "bt709", "unknown", "reserved", "bt470m",
 "bt470bg", "smpte170m", "smpte240m", "film", "bt2020",
+"smpte428_1",
 };
 
 static const char *color_transfer_names[AVCOL_TRC_NB] = {
 "reserved", "bt709", "unknown", "reserved", "bt470m",
 "bt470bg", "smpte170m", "smpte240m", "linear", "log100",
 "log316", "iec61966-2-4", "bt1361e", "iec61966-2-1",
-"bt2020-10", "bt2020-20",
+"bt2020-10", "bt2020-20", "smpte2084", "smpte428-1",
 };
 
 static const char *color_space_names[AVCOL_SPC_NB] = {
-- 
1.7.1

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

Re: [FFmpeg-devel] [PATCH] avutil: Add basic transfer functions for each AVColorTransferCharacteristic

[Kevin Wheatley]

On Tue, Sep 1, 2015 at 12:33 PM, Ronald S. Bultje <rsbul...@gmail.com> wrote:
> Hi,
>
> On Tue, Sep 1, 2015 at 7:02 AM, Kevin Wheatley <kevin.j.wheat...@gmail.com>
> wrote:
>
>> Following on from my previous email, this adds some functions to
>> actually convert from linear to non-linear encoding. I have another
>> that changes the OpenEXR codec to actually use these.
>
>
> Won't performance be crumblingly slow with this kind of API? It seems to
> make more sense to write a filter that can convert between different TRC
> types, or do it in swscale, and then add some fast implementations
> (LUT-based, or approximation-based) alongside the slow versions.

I'm using the functions to construct a LUT in the OpenEXR reader when
possible, the cleanest design would be to pass floating point into the
swscale engine for sure, but I'm not sure I'd personally want to go
changing that code. I'm happy for some other approach to be put
forward, I have some users who wanted the feature, so I have something
that works for their needs so my urgency is minimal.

I've performed a few more tests and spotted something missing in one
of my earlier changes in the pixel description code, patch to
follow...

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

Re: [FFmpeg-devel] [PATCH] avutil: Add basic transfer functions for each AVColorTransferCharacteristic

[Kevin Wheatley]

On Tue, Sep 1, 2015 at 12:49 PM, wm4  wrote:
> Identifiers starting with _ are reserved by the system on file scope.

oh yes, switching between different programming languages never a good
thing for my brain :-)

Would the ffmpeg style be prefixing them up with anything, or just
leaving off the underscore?

Thanks

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

[FFmpeg-devel] [PATCH][RFC] libavformat/mov Extend metadata handling to read in the keys from the 'keys' atom

[Kevin Wheatley]

Looking for feedback on this patch, in particular the use of the
'keys' dictionary within the MOVContext structure.

With the patch FFmpeg can now 'find' the metadata in ARRI's sample
footage see here:

http://www.arri.com/camera/alexa/learn/alexa_sample_footage/

Thanks

Kevin
From 57feef09b636d8d80d3afe7dca20175ddb51 Mon Sep 17 00:00:00 2001
From: Kevin Wheatley <kevin.j.wheat...@gmail.com>
Date: Thu, 3 Sep 2015 15:58:49 +0100
Subject: [PATCH] RFC: Extend metadata handling to read in the keys from the 'keys' atom

---
 libavformat/isom.h |1 +
 libavformat/mov.c  |   63 
 2 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index aee9d6e..8c20ec9 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -206,6 +206,7 @@ typedef struct MOVContext {
 void *audible_fixed_key;
 int audible_fixed_key_size;
 struct AVAES *aes_decrypt;
+AVDictionary *keys_d; // Use a dictionary as an array a little bit ugly
 } MOVContext;
 
 int ff_mp4_read_descr_len(AVIOContext *pb);
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 45367d3..1cf50bb 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -265,6 +265,7 @@ static int mov_read_udta_string(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 uint32_t data_type = 0, str_size, str_size_alloc;
 int (*parse)(MOVContext*, AVIOContext*, unsigned, const char*) = NULL;
 int raw = 0;
+AVDictionaryEntry *entry = NULL;
 
 switch (atom.type) {
 case MKTAG( '@','P','R','M'): key = "premiere_version"; raw = 1; break;
@@ -351,6 +352,13 @@ static int mov_read_udta_string(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 case MKTAG(0xa9,'w','r','n'): key = "warning";   break;
 case MKTAG(0xa9,'w','r','t'): key = "composer";  break;
 case MKTAG(0xa9,'x','y','z'): key = "location";  break;
+default:
+snprintf(key2, sizeof(key2), "%08x", atom.type);
+entry = av_dict_get(c->keys_d, key2, entry, 0);
+if (entry)
+key = entry->value;
+av_log(c->fc, AV_LOG_TRACE, "Attempting to match unknown atom type %08x %s in keys => '%s'\n", atom.type, key2, key);
+break;
 }
 retry:
 if (c->itunes_metadata && atom.size > 8) {
@@ -3184,6 +3192,58 @@ static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 return 0;
 }
 
+static int mov_read_keys(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+AVDictionaryEntry *entry = NULL;
+uint32_t key_count, key_size, key_namespace, key_num;
+char *key_str;
+char key_num_str[8+1] = { 0 }; // Use hexadecimal string of a 32 bit integer + terminator
+int ret;
+
+avio_r8(pb); /* version */
+avio_rb24(pb); /* flags */
+key_count = avio_rb32(pb);
+
+av_log(c->fc, AV_LOG_VERBOSE,
+   "Reading keys sz: %"PRId64" %u\n", atom.size, key_count);
+
+key_num = 1; // Keys are numbered from 1 and refered to in the ilst following.
+while (key_count--) {
+key_size = avio_rb32(pb) - 8;
+key_namespace = avio_rl32(pb);
+if (key_namespace == MKTAG('m','d','t','a')) {
+key_str = av_malloc(key_size + 1); /* Add null terminator */
+if (!key_str) {
+return AVERROR(ENOMEM);
+}
+
+ret = ffio_read_size(pb, key_str, key_size);
+if (ret < 0) {
+av_freep(_str);
+return ret;
+}
+
+key_str[key_size] = 0;
+if (key_str[0]) {
+snprintf(key_num_str, sizeof(key_num_str), "%08x", av_bswap32(key_num));
+av_dict_set(&(c->keys_d), key_num_str, key_str, 0);
+}
+av_freep(_str);
+} else {
+// Don't know what to do with other namespaces... ignore
+avio_seek(pb, key_size, SEEK_CUR);
+}
+++key_num;
+}
+
+while (entry = av_dict_get(c->keys_d, "", entry, AV_DICT_IGNORE_SUFFIX)) {
+av_log(c->fc, AV_LOG_TRACE,
+   "Read metadata key %s: '%s'\n", entry->key, entry->value);
+}
+
+return 0;
+}
+
 static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
 int i;
@@ -3786,6 +3846,7 @@ static const MOVParseTableEntry mov_default_parse_table[] = {
 { MKTAG('h','d','l','r'), mov_read_hdlr },
 { MKTAG('i','l','s','t'), mov_read_ilst },
 { MKTAG('j','p','2','h'), mov_read_jp2h },
+{ MKTAG('k','e','y','s'), mov_read_keys },
 { MKTAG('m','d','a','t'), mov_read_mdat },
 { MKTAG('m','d','h','d'), mov_read_mdhd },
 { MKTAG('m','d','i','a'), mov_read_default },
@@ -4203,6 +4264,8 @@ static int mov_read_close(AVFormatContext *s)
 av_freep(>fragment_index_data);
 
 av_freep(>aes_decrypt);
+
+av_dict_free(

Re: [FFmpeg-devel] [PATCH][RFC] libavformat/mov Extend metadata handling to read in the keys from the 'keys' atom

[Kevin Wheatley]

On Thu, Sep 3, 2015 at 7:49 PM, Michael Niedermayer  wrote:
> missing checks for interger overflows of the addition and subtraction
>
> also the subject says "RFC", is there a reason not to push this to
> git master once it otherwise looks good ?


it is incomplete, basically I was fishing for a general OK to (ab)use
the dictionary in the MOVContext as a means to pass on the list of
keys, having done that, I've moved on to extend the data types
understood to include other data types (The sample ARRI clips need
signed and unsigned integers). I have this working but the code is in
need of a refactoring to say the least.

There does not appear to be a way to indicate the 'type' of data
pushed into the dictionary so that if I wanted to serialise it back
out I could get the same format, is this something people have
considered?

I could use a type dictionary to store the original type of the keys
along side the standard metatdata dictionary, but how that would feed
into the metatdata mux/demuxing and the command line interface I don't
know.

Thanks

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

[FFmpeg-devel] [PATCH] libavutil/dict: extend the list of convienience functions for storing different data types

[Kevin Wheatley]

Hi,

as part of adding support for non-string data types to .mov metadata,
I wondered about adding the following helper functions for storing
numeric types into an AVDictionary.

upfront I'll say I'm not 100% happy with the float32 and float64 named
variants (vs float and double) as there is no clear preference in
other areas of the code that I could see.

I'm also conscious that to provide for rewriting the .mov metadata
types, I'll need something different to store the actual types and
that might mean these functions would end up not being used at all.

Kevin
From 34fedb67d58402b519a7c91bff7623469802c4c4 Mon Sep 17 00:00:00 2001
From: Kevin Wheatley <kevin.j.wheat...@gmail.com>
Date: Fri, 4 Sep 2015 14:26:49 +0100
Subject: [PATCH] libavutil/dict: extend the list of convienience functions for storing different data types


Signed-off-by: Kevin Wheatley <kevin.j.wheat...@gmail.com>
---
 libavutil/dict.c|   81 +-
 libavutil/dict.h|   24 +++
 tests/ref/fate/dict |   17 +++
 3 files changed, 120 insertions(+), 2 deletions(-)

diff --git a/libavutil/dict.c b/libavutil/dict.c
index 6ff1af5..4eaaf83 100644
--- a/libavutil/dict.c
+++ b/libavutil/dict.c
@@ -149,6 +149,33 @@ int av_dict_set_int(AVDictionary **pm, const char *key, int64_t value,
 return av_dict_set(pm, key, valuestr, flags);
 }
 
+int av_dict_set_uint(AVDictionary **pm, const char *key, uint64_t value,
+int flags)
+{
+char valuestr[22];
+snprintf(valuestr, sizeof(valuestr), "%"PRIu64, value);
+flags &= ~AV_DICT_DONT_STRDUP_VAL;
+return av_dict_set(pm, key, valuestr, flags);
+}
+
+int av_dict_set_float32(AVDictionary **pm, const char *key, float value,
+int flags)
+{
+char valuestr[17]; // sign + 1 + point + 8 + e + sign + 3 + term
+snprintf(valuestr, sizeof(valuestr), "%.9g", value);
+flags &= ~AV_DICT_DONT_STRDUP_VAL;
+return av_dict_set(pm, key, valuestr, flags);
+}
+
+int av_dict_set_float64(AVDictionary **pm, const char *key, double value,
+int flags)
+{
+char valuestr[26]; // sign + 1 + point + 16 + e + sign + 4 + term
+snprintf(valuestr, sizeof(valuestr), "%.17g", value);
+flags &= ~AV_DICT_DONT_STRDUP_VAL;
+return av_dict_set(pm, key, valuestr, flags);
+}
+
 static int parse_key_value_pair(AVDictionary **pm, const char **buf,
 const char *key_val_sep, const char *pairs_sep,
 int flags)
@@ -276,9 +303,13 @@ static void test_separators(const AVDictionary *m, const char pair, const char v
 int main(void)
 {
 AVDictionary *dict = NULL;
-AVDictionaryEntry *e;
+AVDictionaryEntry *e, *e2;
 char *buffer = NULL;
-
+float f32 = 0.0f;
+double f64 = 0.0f;
+union { uint32_t i; float f; } intfloat;
+union { uint64_t ui; int64_t i; } un_signed_int;
+
 printf("Testing av_dict_get_string() and av_dict_parse_string()\n");
 av_dict_get_string(dict, , '=', ',');
 printf("%s\n", buffer);
@@ -356,6 +387,52 @@ int main(void)
 printf("%s\n", e->value);
 av_dict_free();
 
+printf("\nTesting av_dict_set_int()\n");
+un_signed_int.ui = 0U;
+av_dict_set_int(, "0", un_signed_int.i, 0);
+un_signed_int.ui = 0x8000UL;
+av_dict_set_int(, "-max", un_signed_int.i, 0);
+un_signed_int.ui = 0x7fffUL;
+av_dict_set_int(, "max", un_signed_int.i, 0);
+e = NULL;
+while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
+printf("%s %s\n", e->key, e->value);
+av_dict_free();
+
+printf("\nTesting av_dict_set_uint()\n");
+av_dict_set_uint(, "0", 0, 0);
+av_dict_set_uint(, "max", 0xUL, 0);
+e = NULL;
+while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
+printf("%s %s\n", e->key, e->value);
+av_dict_free();
+
+printf("\nTesting av_dict_set_float*()\n");
+// float and double representation of a given number should give the same value
+f32 = -1.20786635e-05f;
+f64 = -1.20786635e-05;
+av_dict_set_float32(, "f32", f32, 0);
+e = av_dict_get(dict, "f32", NULL, 0);
+printf("%.30g => %s\n", f32, e->value);
+av_dict_set_float64(, "f64", f64, 0);
+e2 = av_dict_get(dict, "f64", NULL, 0);
+printf("%.30g => %s\n", f64, e2->value);
+printf("%s == %s %s\n", e->value, e2->value, strcmp(e->value, e2->value) == 0 ? "OK" : "BAD");
+
+intfloat.i = 0xa647609;
+av_dict_set_float32(, "0xa647609", intfloat.f, 0);
+e = av_dict_get(dict, "0xa647609", NULL, 0);
+printf("%.30g =&

Re: [FFmpeg-devel] [PATCH] libavutil/dict: extend the list of convienience functions for storing different data types

[Kevin Wheatley]

On Fri, Sep 4, 2015 at 4:18 PM, wm4 <nfx...@googlemail.com> wrote:
> On Fri, 4 Sep 2015 14:38:54 +0100
> Kevin Wheatley <kevin.j.wheat...@gmail.com> wrote:
>
>> Hi,
>>
>> as part of adding support for non-string data types to .mov metadata,
>> I wondered about adding the following helper functions for storing
>> numeric types into an AVDictionary.
>>
>> upfront I'll say I'm not 100% happy with the float32 and float64 named
>> variants (vs float and double) as there is no clear preference in
>> other areas of the code that I could see.
>
> I don't understand it either. Why not just use float and double,
> instead of obfuscating the names further? (It'd be a difference if this
> function e.g. serialized the floats to binary - but it literally just
> passes them as-is to libc. Having the C data type in the argument seems
> advantageous.
>
> Also, what's the use in having a float function at all?
>
> I'd call av_dict_set_uint av_dict_set_uint64.

I'll explain my POV...

Essentially these helpers do the same as the existing
av_dict_set_int() does for int64_t so I've essentially just copied
that and substituted the required conversions from  to char
array. The floating point is needed to correctly decode those formats.

Using the bit length in the name of the floating point variation of
the functions is because they will only correctly produce the textual
representation sufficient to transform back into the same binary
representation based on the bit lengths of the floating point
representation (they assume IEEE single and double precision in the
lengths of the formatting options and thus buffer size).

Mentally I was in the middle of the Apple QuickTime metadata
representations where they also use the floatXX terminology - not a
great excuse I know :-)


Re Nicholas' comments on the

flags &= ~AV_DICT_DONT_STRDUP_VAL;

I was simply replicating the existing form of the function av_dict_set_int().

As to portability in the tests - yes I agree - happy for somebody to
point me towards a good solution for that.


My alternative to these would have been to embed the formatting in the
libavformat/mov.c code which obviously would have less impact.

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

Re: [FFmpeg-devel] [PATCH][RFC] libavformat/mov Extend metadata handling to read in the keys from the 'keys' atom

[Kevin Wheatley]

I've updated and extended the metadata reading to include some of the
numeric types, based on the feedback from the comments about adding to
the utility functions of AVDictionary, I've directly formatted the
values as strings in the .mov format code seams less risky.

Kevin
From 91515aec2e964e0fb499378e31cd7782bf93482a Mon Sep 17 00:00:00 2001
From: Kevin Wheatley <kevin.j.wheat...@gmail.com>
Date: Thu, 3 Sep 2015 15:58:49 +0100
Subject: [PATCH] avformat/mov: Extend metadata handling to read in the keys from the 'keys' atom

Signed-off-by: Kevin Wheatley <kevin.j.wheat...@gmail.com>
---
 libavformat/isom.h |1 +
 libavformat/mov.c  |   63 
 2 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index aee9d6e..8c20ec9 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -206,6 +206,7 @@ typedef struct MOVContext {
 void *audible_fixed_key;
 int audible_fixed_key_size;
 struct AVAES *aes_decrypt;
+AVDictionary *keys_d; // Use a dictionary as an array a little bit ugly
 } MOVContext;
 
 int ff_mp4_read_descr_len(AVIOContext *pb);
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 92d90db..46c4b17 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -265,6 +265,7 @@ static int mov_read_udta_string(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 uint32_t data_type = 0, str_size, str_size_alloc;
 int (*parse)(MOVContext*, AVIOContext*, unsigned, const char*) = NULL;
 int raw = 0;
+AVDictionaryEntry *entry = NULL;
 
 switch (atom.type) {
 case MKTAG( '@','P','R','M'): key = "premiere_version"; raw = 1; break;
@@ -351,6 +352,13 @@ static int mov_read_udta_string(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 case MKTAG(0xa9,'w','r','n'): key = "warning";   break;
 case MKTAG(0xa9,'w','r','t'): key = "composer";  break;
 case MKTAG(0xa9,'x','y','z'): key = "location";  break;
+default:
+snprintf(key2, sizeof(key2), "%08x", atom.type);
+entry = av_dict_get(c->keys_d, key2, entry, 0);
+if (entry)
+key = entry->value;
+av_log(c->fc, AV_LOG_TRACE, "Attempting to match unknown atom type %08x %s in keys => '%s'\n", atom.type, key2, key);
+break;
 }
 retry:
 if (c->itunes_metadata && atom.size > 8) {
@@ -3184,6 +3192,58 @@ static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 return 0;
 }
 
+static int mov_read_keys(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+AVDictionaryEntry *entry = NULL;
+uint32_t key_count, key_size, key_namespace, key_num;
+char *key_str;
+char key_num_str[8+1] = { 0 }; // Use hexadecimal string of a 32 bit integer + terminator
+int ret;
+
+avio_r8(pb); /* version */
+avio_rb24(pb); /* flags */
+key_count = avio_rb32(pb);
+
+av_log(c->fc, AV_LOG_VERBOSE,
+   "Reading keys sz: %"PRId64" %u\n", atom.size, key_count);
+
+key_num = 1; // Keys are numbered from 1 and refered to in the ilst following.
+while (key_count--) {
+key_size = avio_rb32(pb) - 8;
+key_namespace = avio_rl32(pb);
+if (key_namespace == MKTAG('m','d','t','a')) {
+key_str = av_malloc(key_size + 1); /* Add null terminator */
+if (!key_str) {
+return AVERROR(ENOMEM);
+}
+
+ret = ffio_read_size(pb, key_str, key_size);
+if (ret < 0) {
+av_freep(_str);
+return ret;
+}
+
+key_str[key_size] = 0;
+if (key_str[0]) {
+snprintf(key_num_str, sizeof(key_num_str), "%08x", av_be2ne32(key_num));
+av_dict_set(&(c->keys_d), key_num_str, key_str, 0);
+}
+av_freep(_str);
+} else {
+// Don't know what to do with other namespaces... ignore
+avio_seek(pb, key_size, SEEK_CUR);
+}
+++key_num;
+}
+
+while (entry = av_dict_get(c->keys_d, "", entry, AV_DICT_IGNORE_SUFFIX)) {
+av_log(c->fc, AV_LOG_TRACE,
+   "Read metadata key %s: '%s'\n", entry->key, entry->value);
+}
+
+return 0;
+}
+
 static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
 int i;
@@ -3786,6 +3846,7 @@ static const MOVParseTableEntry mov_default_parse_table[] = {
 { MKTAG('h','d','l','r'), mov_read_hdlr },
 { MKTAG('i','l','s','t'), mov_read_ilst },
 { MKTAG('j','p','2','h'), mov_read_jp2h },
+{ MKTAG('k','e','y','s'), mov_read_keys },
 { MKTAG('m','d','a','t'), mov_read_mdat },
 { MKTAG('m','d','h','d'), mov_read_mdhd },
 { MKTAG('m','d','i','a'), mov_read_default },
@@ -4203,6 +4264,8 @@ static int mov_read_close(AVFormatContext *s)
 av_freep(>fragment_ind

Re: [FFmpeg-devel] [PATCH] avutil: Add basic transfer functions for each AVColorTransferCharacteristic

[Kevin Wheatley]

Rather than rewrite history or anything like that people could either
apply this on top of my previous patches, or for a view of the whole
kit and kaboodle:

https://github.com/FFmpeg/FFmpeg/compare/master...KevinJW:image_exr_transfer_characteristics

Happy to take further feedback,

Kevin

On Tue, Sep 1, 2015 at 1:03 PM, wm4 <nfx...@googlemail.com> wrote:
> On Tue, 1 Sep 2015 12:55:49 +0100
> Kevin Wheatley <kevin.j.wheat...@gmail.com> wrote:
>
>> On Tue, Sep 1, 2015 at 12:49 PM, wm4 <nfx...@googlemail.com> wrote:
>> > Identifiers starting with _ are reserved by the system on file scope.
>>
>> oh yes, switching between different programming languages never a good
>> thing for my brain :-)
>>
>> Would the ffmpeg style be prefixing them up with anything, or just
>> leaving off the underscore?
>
> Yes. For static functions, any identifier should be fine; for
> identifiers which are not static and are needed in other files,
> but are still library-private, we use a ff_ prefix.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
From 313d6f147b1beb05ab5676d8cd0a7c745c60051d Mon Sep 17 00:00:00 2001
From: Kevin Wheatley <kevin.j.wheat...@gmail.com>
Date: Tue, 1 Sep 2015 14:02:31 +0100
Subject: [PATCH 8/8] avoid the use of reserved names

---
 libavutil/color_utils.c |   48 +++---
 1 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/libavutil/color_utils.c b/libavutil/color_utils.c
index f84b5f5..81ba447 100644
--- a/libavutil/color_utils.c
+++ b/libavutil/color_utils.c
@@ -57,7 +57,7 @@ double avpriv_get_gamma_from_trc(enum AVColorTransferCharacteristic trc)
 #define BT709_alpha 1.099296826809442
 #define BT709_beta 0.018053968510807
 
-static double _trc_bt709(double Lc)
+static double avpriv_trc_bt709(double Lc)
 {
 const double a = BT709_alpha;
 const double b = BT709_beta;
@@ -67,17 +67,17 @@ static double _trc_bt709(double Lc)
  :  a * pow(Lc, 0.45) - (a - 1.0);
 }
 
-static double _trc_gamma22(double Lc)
+static double avpriv_trc_gamma22(double Lc)
 {
 return (0.0 > Lc) ? 0.0 : pow(Lc, 1.0/ 2.2);
 }
 
-static double _trc_gamma28(double Lc)
+static double avpriv_trc_gamma28(double Lc)
 {
 return (0.0 > Lc) ? 0.0 : pow(Lc, 1.0/ 2.8);
 }
 
-static double _trc_smpte240M(double Lc)
+static double avpriv_trc_smpte240M(double Lc)
 {
 const double a = 1.1115;
 const double b = 0.0228;
@@ -87,23 +87,23 @@ static double _trc_smpte240M(double Lc)
  :  a * pow(Lc, 0.45) - (a - 1.0);
 }
 
-static double _trc_linear(double Lc)
+static double avpriv_trc_linear(double Lc)
 {
 return Lc;
 }
 
-static double _trc_log(double Lc)
+static double avpriv_trc_log(double Lc)
 {
 return (0.01 > Lc) ? 0.0 : 1.0 + log10(Lc) / 2.0;
 }
 
-static double _trc_log_sqrt(double Lc)
+static double avpriv_trc_log_sqrt(double Lc)
 {
 // sqrt(10) / 1000
 return (0.00316227766 > Lc) ? 0.0 : 1.0 + log10(Lc) / 2.5;
 }
 
-static double _trc_iec61966_2_4(double Lc)
+static double avpriv_trc_iec61966_2_4(double Lc)
 {
 const double a = BT709_alpha;
 const double b = BT709_beta;
@@ -113,7 +113,7 @@ static double _trc_iec61966_2_4(double Lc)
  :   a * pow( Lc, 0.45) - (a - 1.0);
 }
 
-static double _trc_bt1361(double Lc)
+static double avpriv_trc_bt1361(double Lc)
 {
 const double a = BT709_alpha;
 const double b = BT709_beta;
@@ -123,7 +123,7 @@ static double _trc_bt1361(double Lc)
  :   a * pow( Lc, 0.45) - (a - 1.0);
 }
 
-static double _trc_iec61966_2_1(double Lc)
+static double avpriv_trc_iec61966_2_1(double Lc)
 {
 const double a = 1.055;
 const double b = 0.0031308;
@@ -133,7 +133,7 @@ static double _trc_iec61966_2_1(double Lc)
  :  a * pow(Lc, 1.0  / 2.4) - (a - 1.0);
 }
 
-static double _trc_smpte_st2084(double Lc)
+static double avpriv_trc_smpte_st2084(double Lc)
 {
 const double c1 = 3424.0 / 4096.0; // c3-c2 + 1
 const double c2 =  32.0 * 2413.0 / 4096.0;
@@ -148,7 +148,7 @@ static double _trc_smpte_st2084(double Lc)
 
 }
 
-static double _trc_smpte_st428_1(double Lc)
+static double avpriv_trc_smpte_st428_1(double Lc)
 {
 return (0.0 > Lc) ? 0.0
  :  pow(48.0 * Lc / 52.37, 1.0 / 2.6);
@@ -162,50 +162,50 @@ avpriv_trc_function avpriv_get_trc_function_from_trc(enum AVColorTransferCharact
 case AVCOL_TRC_SMPTE170M:
 case AVCOL_TRC_BT2020_10:
 case AVCOL_TRC_BT2020_12:
-func = _trc_bt709;
+func = avpriv_trc_bt709;
 break;
 
 case AVCOL_TRC_GAMMA22:
-func = _trc_gamma22;
+func = avpriv_trc_gamma22;
 break;
 case AVCOL_TRC_GAMMA28:
-func = _trc_

[FFmpeg-devel] [PATCH] avcodec/exr/c Add support for applying a transfer characteristic curve to OpenEXR inputs.

[Kevin Wheatley]

This adds the actual usage and allows for command lines similar to this:

ffmpeg -apply_trc bt709 -i linear.exr bt709.png
ffmpeg -apply_trc smpte2084 -i linear.exr smpte2084.png
ffmpeg -apply_trc iec61966_2_1 -i linear.exr sRGB.png


Which assuming the correct primaries in the linear OpenEXR file will
generate the appropriate file.

Kevin
From 25aedae9dc873abcba3a6db3dff503c64cd9e066 Mon Sep 17 00:00:00 2001
From: Kevin Wheatley <kevin.j.wheat...@gmail.com>
Date: Tue, 1 Sep 2015 11:02:53 +0100
Subject: [PATCH 5/5] Add support for applying a transfer characteristic curve to OpenEXR inputs.

Signed-off-by: Kevin Wheatley <kevin.j.wheat...@gmail.com>
---
 libavcodec/exr.c |  123 ++
 1 files changed, 96 insertions(+), 27 deletions(-)

diff --git a/libavcodec/exr.c b/libavcodec/exr.c
index b9de7c1..8794da5 100644
--- a/libavcodec/exr.c
+++ b/libavcodec/exr.c
@@ -37,6 +37,7 @@
 #include "libavutil/imgutils.h"
 #include "libavutil/intfloat.h"
 #include "libavutil/opt.h"
+#include "libavutil/color_utils.h"
 
 #include "avcodec.h"
 #include "bytestream.h"
@@ -110,6 +111,7 @@ typedef struct EXRContext {
 
 const char *layer;
 
+enum AVColorTransferCharacteristic apply_trc_type;
 float gamma;
 uint16_t gamma_table[65536];
 } EXRContext;
@@ -842,6 +844,7 @@ static int decode_block(AVCodecContext *avctx, void *tdata,
 int bxmin = s->xmin * 2 * s->desc->nb_components;
 int i, x, buf_size = s->buf_size;
 float one_gamma = 1.0f / s->gamma;
+avpriv_trc_function trc_func = avpriv_get_trc_function_from_trc(s->apply_trc_type);
 int ret;
 
 line_offset = AV_RL64(s->gb.buffer + jobnr * 8);
@@ -921,24 +924,43 @@ static int decode_block(AVCodecContext *avctx, void *tdata,
 ptr_x += s->xmin * s->desc->nb_components;
 if (s->pixel_type == EXR_FLOAT) {
 // 32-bit
-for (x = 0; x < xdelta; x++) {
-union av_intfloat32 t;
-t.i = bytestream_get_le32();
-if (t.f > 0.0f)  /* avoid negative values */
-t.f = powf(t.f, one_gamma);
-*ptr_x++ = exr_flt2uint(t.i);
-
-t.i = bytestream_get_le32();
-if (t.f > 0.0f)
-t.f = powf(t.f, one_gamma);
-*ptr_x++ = exr_flt2uint(t.i);
-
-t.i = bytestream_get_le32();
-if (t.f > 0.0f)
-t.f = powf(t.f, one_gamma);
-*ptr_x++ = exr_flt2uint(t.i);
-if (channel_buffer[3])
-*ptr_x++ = exr_flt2uint(bytestream_get_le32());
+if (trc_func) {
+for (x = 0; x < xdelta; x++) {
+union av_intfloat32 t;
+t.i = bytestream_get_le32();
+t.f = trc_func(t.f);
+*ptr_x++ = exr_flt2uint(t.i);
+
+t.i = bytestream_get_le32();
+t.f = trc_func(t.f);
+*ptr_x++ = exr_flt2uint(t.i);
+
+t.i = bytestream_get_le32();
+t.f = trc_func(t.f);
+*ptr_x++ = exr_flt2uint(t.i);
+if (channel_buffer[3])
+*ptr_x++ = exr_flt2uint(bytestream_get_le32());
+}
+} else {
+for (x = 0; x < xdelta; x++) {
+union av_intfloat32 t;
+t.i = bytestream_get_le32();
+if (t.f > 0.0f)  /* avoid negative values */
+t.f = powf(t.f, one_gamma);
+*ptr_x++ = exr_flt2uint(t.i);
+
+t.i = bytestream_get_le32();
+if (t.f > 0.0f)
+t.f = powf(t.f, one_gamma);
+*ptr_x++ = exr_flt2uint(t.i);
+
+t.i = bytestream_get_le32();
+if (t.f > 0.0f)
+t.f = powf(t.f, one_gamma);
+*ptr_x++ = exr_flt2uint(t.i);
+if (channel_buffer[3])
+*ptr_x++ = exr_flt2uint(bytestream_get_le32());
+}
 }
 } else {
 // 16-bit
@@ -1364,21 +1386,31 @@ static av_cold int decode_init(AVCodecContext *avctx)
 uint32_t i;
 union av_intfloat32 t;
 float one_gamma = 1.0f / s->gamma;
+avpriv_trc_function trc_func = NULL;
 
 s->avctx  = avctx;
 
-if (one_gamma > 0.f && one_gamma < 1.0001f) {
-for (i = 0; i < 65536; ++i)
-s->gamma_table[i] = exr_halflt2uint(i);
-} else {
+trc_func = avpriv_get_trc_function_from_trc(s->apply_trc_type);
+if (trc_func) {
 for (i = 0; i < 65536; ++i) {
 t = exr_half2float(i);
-   

Re: [FFmpeg-devel] [PATCH] avcodec/exr/c Add support for applying a transfer characteristic curve to OpenEXR inputs.

[Kevin Wheatley]

This actually marks up the buffers as having the state given by the applied trc,

Kevin

On Tue, Sep 1, 2015 at 12:04 PM, Kevin Wheatley
<kevin.j.wheat...@gmail.com> wrote:
> This adds the actual usage and allows for command lines similar to this:
>
> ffmpeg -apply_trc bt709 -i linear.exr bt709.png
> ffmpeg -apply_trc smpte2084 -i linear.exr smpte2084.png
> ffmpeg -apply_trc iec61966_2_1 -i linear.exr sRGB.png
>
>
> Which assuming the correct primaries in the linear OpenEXR file will
> generate the appropriate file.
>
> Kevin
From 0e6bd2437146dafef0e6a89c9db378ba534211bd Mon Sep 17 00:00:00 2001
From: Kevin Wheatley <kevin.j.wheat...@gmail.com>
Date: Tue, 1 Sep 2015 12:35:55 +0100
Subject: [PATCH 7/7] Mark up the decoded buffer as the appropriate transfer characteristic when applying one

---
 libavcodec/exr.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/libavcodec/exr.c b/libavcodec/exr.c
index 8794da5..92f528a 100644
--- a/libavcodec/exr.c
+++ b/libavcodec/exr.c
@@ -1308,6 +1308,9 @@ static int decode_frame(AVCodecContext *avctx, void *data,
 av_log(avctx, AV_LOG_ERROR, "Missing channel list.\n");
 return AVERROR_INVALIDDATA;
 }
+
+if (s->apply_trc_type != AVCOL_TRC_UNSPECIFIED)
+avctx->color_trc = s->apply_trc_type;
 
 switch (s->compression) {
 case EXR_RAW:
-- 
1.7.1

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

[FFmpeg-devel] Support of QuickTime metadata atoms

[Kevin Wheatley]

Hi,

I'm considering adding support for writing the QuickTime metadata atom
structure to support arbitrary key value pairs, similar to those
included by ARRI cameras see:

https://developer.apple.com/library/prerelease/mac/documentation/QuickTime/QTFF/qtff.pdf

p128

To start I was going to try add reading of the metadata, as such I was
wondering about FFmpeg's current metadata handling and how to add the
metadata found in these atoms to the internal structures.

Nominally these metadata atoms can appear in multiple places within
the QuickTime file.

In particular I wondered if there is any need for name spacing the
keys similar to the way the QuickTime specifications use the 'mdta'
handler field both as a way to distinguish from the current 'well
known named' metadata as well as to potentially support multiple sets
of metadata from different parts of the atom tree hierarchy (I haven't
worked out if there is a 1:1 mapping with the way ffmpeg currently
stores metadata vs potential positions in the atom tree)

Has anybody considered this previously and could point me in the right
direction?

Thanks

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

Re: [FFmpeg-devel] [PATCH] avfilter: add zscale filter

[Kevin Wheatley]

On Wed, Sep 30, 2015 at 9:49 AM, Paul B Mahol  wrote:
> +{ "range", "set color range",  OFFSET(range), AV_OPT_TYPE_INT, 
> {.i64 = -1}, -1, ZIMG_RANGE_FULL, FLAGS, "range" },
> +{ "r", "set color range",  OFFSET(range), AV_OPT_TYPE_INT, 
> {.i64 = -1}, -1, ZIMG_RANGE_FULL, FLAGS, "range" },
> +{ "input",0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = -1}, 0, 0, FLAGS, "range" },
> +{ "limited",  0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_RANGE_LIMITED}, 0, 0, FLAGS, "range" },
> +{ "full", 0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_RANGE_FULL},0, 0, FLAGS, "range" },
> +{ "primaries", "set color primaries", OFFSET(primaries), 
> AV_OPT_TYPE_INT, {.i64 = -1}, -1, ZIMG_PRIMARIES_2020, FLAGS, "primaries" },
> +{ "p", "set color primaries", OFFSET(primaries), 
> AV_OPT_TYPE_INT, {.i64 = -1}, -1, ZIMG_PRIMARIES_2020, FLAGS, "primaries" },
> +{ "input",0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = -1}, 0, 0, FLAGS, "primaries" },
> +{ "709",  0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_PRIMARIES_709}, 0, 0, FLAGS, "primaries" },
> +{ "unspecified",  0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_PRIMARIES_UNSPECIFIED}, 0, 0, FLAGS, "primaries" },
> +{ "170m", 0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_PRIMARIES_170M},0, 0, FLAGS, "primaries" },
> +{ "240m", 0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_PRIMARIES_240M},0, 0, FLAGS, "primaries" },
> +{ "2020", 0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_PRIMARIES_2020},0, 0, FLAGS, "primaries" },
> +{ "transfer", "set transfer characteristic", OFFSET(trc), 
> AV_OPT_TYPE_INT, {.i64 = -1}, -1, ZIMG_TRANSFER_2020_12, FLAGS, "transfer" },
> +{ "t","set transfer characteristic", OFFSET(trc), 
> AV_OPT_TYPE_INT, {.i64 = -1}, -1, ZIMG_TRANSFER_2020_12, FLAGS, "transfer" },
> +{ "input",0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = -1}, 0, 0, FLAGS, "transfer" },
> +{ "709",  0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_TRANSFER_709}, 0, 0, FLAGS, "transfer" },
> +{ "unspecified",  0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_TRANSFER_UNSPECIFIED}, 0, 0, FLAGS, "transfer" },
> +{ "601",  0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_TRANSFER_601}, 0, 0, FLAGS, "transfer" },
> +{ "linear",   0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_TRANSFER_LINEAR},  0, 0, FLAGS, "transfer" },
> +{ "2020_10",  0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_TRANSFER_2020_10}, 0, 0, FLAGS, "transfer" },
> +{ "2020_12",  0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_TRANSFER_2020_12}, 0, 0, FLAGS, "transfer" },
> +{ "matrix", "set colorspace matrix", OFFSET(colorspace), 
> AV_OPT_TYPE_INT, {.i64 = -1}, -1, ZIMG_MATRIX_2020_CL, FLAGS, "matrix" },
> +{ "m",  "set colorspace matrix", OFFSET(colorspace), 
> AV_OPT_TYPE_INT, {.i64 = -1}, -1, ZIMG_MATRIX_2020_CL, FLAGS, "matrix" },
> +{ "input",0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = -1}, 0, 0, FLAGS, "matrix" },
> +{ "709",  0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_MATRIX_709},0, 0, FLAGS, "matrix" },
> +{ "unspecified",  0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_MATRIX_UNSPECIFIED},0, 0, FLAGS, "matrix" },
> +{ "470bg",0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_MATRIX_470BG},  0, 0, FLAGS, "matrix" },
> +{ "170m", 0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_MATRIX_170M},   0, 0, FLAGS, "matrix" },
> +{ "2020_ncl", 0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_MATRIX_2020_NCL},   0, 0, FLAGS, "matrix" },
> +{ "2020_cl",  0,   0, AV_OPT_TYPE_CONST, 
> {.i64 = ZIMG_MATRIX_2020_CL},0, 0, FLAGS, "matrix" },


As a casual developer/observer, I wonder about the consistency of the
options here with other parts of ffmpeg, in the general case it uses:

  -color_primaries
 bt709
 unspecified
 bt470m
 bt470bg
 smpte170m
 smpte240m
 film
 bt2020
  -color_trc
 bt709
 unspecified
 gamma22
 gamma28
 smpte170m
 smpte240m
 linear
 log
 log_sqrt
 iec61966_2_4
 bt1361
 iec61966_2_1
 bt2020_10bit
 bt2020_12bit
  

Re: [FFmpeg-devel] avformat/mov: improve qt metadata reading and writing

[Kevin Wheatley]

On Thu, Jun 23, 2016 at 10:44 AM, Michael Niedermayer
 wrote:
> that was maybe forgotten due to the rfc in the subject

I never pushed for it as our internal requirement for the feature went
away and I only try to push things we actually found useful :-) the
patch probably won't merge cleanly now anyway.

The other part of the problem is ideally I'd like to output the
metadata correctly, but to do that ffmpeg would need to understand
multiple data types better (or at least when I looked at it last this
was the case). I also remember there maybe similar comments about
libav needing more robust metadata support too.

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

Re: [FFmpeg-devel] Avid DNxHR / UK DPP

[Kevin Wheatley]

On Tue, Jan 26, 2016 at 5:30 PM, John Warburton  wrote:
> I'm at DPP conference, in the room. Start of UHD delivery standards for UK
> just announced. Now on website, we are told.

https://www.digitalproductionpartnership.co.uk/what-we-do/technical-standards/

http://dpp-assets.s3.amazonaws.com/wp-content/uploads/2016/01/TechnicalDeliverySupplementUHDDPP.pdf

is the template for broadcasters to fill in the blanks.

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

Re: [FFmpeg-devel] [PATCH 3/3] vf_colorspace: Add support for smpte 431/432 (dci/display p3) primaries

[Kevin Wheatley]

On Sun, Oct 30, 2016 at 1:18 PM, Ronald S. Bultje  wrote:
> Hmm... So, the wikipedia page https://en.wikipedia.org/wiki/DCI-P3 refers
> to the two whitepoints here as DCI-P3 D65 and DCI-P3 Theater. Calling one
> D65 and the other DCI seems confusing in that light (assuming the wikipedia
> page is correct). I'd call it THEATER or DCI_P3_THEATER, to distinguish it
> from DCI-P3 D65. Is that OK?

In the industry people just call it the DCI P3 white point (or 'Urgh')
it is not limited to theater usage, you might consider it the
calibration white point for the reference projector, so
WP_DCI_P3_REFERENCE might be better, but that is a little long.

I'd go for something like WP_DCI_P3 it is not really ambiguous.

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

Re: [FFmpeg-devel] [PATCH 3/3] vf_colorspace: Add support for smpte 431/432 (dci/display p3) primaries

[Kevin Wheatley]

I would really strongly suggest including DCI in the name at least -
though nobody else would choose to use it for anything other than the
reference calibration - most titles use a creative white different to
that of the encoding reference (one that is less green).

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

Re: [FFmpeg-devel] [PATCH] libavcodec/exr: Fix blank output when data window != display window

[Kevin Wheatley]

No, just this kind.

Behind the images when data = display window, the most common case
OpenEXR file we have, is with a reduced data window that resides
completely inside the display window, though this particular bug would
impact any files where the value of ymax+1 is not equal to the height.

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

[FFmpeg-devel] [PATCH] libavcodec/exr: Fix blank output when data window != display window

[Kevin Wheatley]

looks like there is a bug in commit
1a08758e7c4e14a9ea8d2fef6c33ad411b2d3c40 relating to the handling of
ptr in decode_frame after decode_block is called, before this commit
ptr would have been incremented for each line in the data window, now
after the commit it is left at the start of the first included line
rather than the line after the data window then the code sets the
remaining lines to 0 and thus the whole image is over written.

Fix by adjusting ptr to the correct line after decode_block returns

Signed-off-by: Kevin Wheatley <kevin.j.wheat...@gmail.com>


0001-libavcodec-exr-Fix-blank-output-when-data-window-dis.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libavcodec/exr: Fix blank output when data window != display window

[Kevin Wheatley]

The following is a sample EXR that needs the patch to correctly
process in FFmpeg.

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

Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Kevin Wheatley]

The guess work comes from this list:

https://developer.apple.com/library/content/technotes/tn2162/_index.html#//apple_ref/doc/uid/DTS40013070-CH1-TNTAG10-SAMPLE__COLR__SETTINGS

and I agree is the source of a lot of chaos, but it is what several
tools have done in the absence of any specified values.

My usage of the feature is to write the atom when I need to by
explicitly enabling it on the command line and I also explicitly set
the metadata either from the input file, or by overriding on the
command line.

Kevin


On Mon, Nov 20, 2017 at 4:52 PM, Derek Buitenhuis
 wrote:
> On 11/20/2017 4:45 PM, Dave Rice wrote:
>> What do you propose as the default for guessed_hacky_crap? Also are there 
>> supporters for the need of a guessed_hacky_crap optio? Is there precedence 
>> in ffmpeg to enable/disable guesswork via a user option?
>
> I personally would never use the guesswork stuff, since I think it is a
> terrible idea, so can't really propose a default or support it.
>
> The guesswork stuff was added at the same time as colr support, by Michael,
> in 7b6f4191763a5ffde02fa198101aabc3ddb5bf61, so maybe he has some opinion.
>
> - Derek
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libavfilter/vf_libvmaf.c The libvmaf filter tried to join on an invalid thread id

[Kevin Wheatley]

Following up my own email with another question or so:

Could somebody point me at a suitable method of testing this within
the Fate framework?

I've started to try unpick how the fate tests are run, etc

What I need is a prototypical example which presumably would need to
factor in (what I figured out so far):

libvmaf is an optional component so the test should be too.
The test should probably live in tests/fate/filter-video.mak
The function (CMD) called should live in tests/fate-run.sh?

For the actual test I need two inputs an original and an encoded frame
(sequence is ok too), that can then feed into the vmaf invocation of
ffmpeg. Is there a set of inputs I can just reuse from what fate would
normally run so that the test need only run ffmpeg to compute the
output of vmaf, (or is the preference for every test to generate its
own input?)

Thanks

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

[FFmpeg-devel] [PATCH] libavfilter/vf_libvmaf.c The libvmaf filter tried to join on an invalid thread id

[Kevin Wheatley]

Following on from my report in the user list here is a better than a
quick hack suggestion to avoid trying to join on an invalid thread id.
This may not be the best solution, but it does avoid the SEGV on linux
when calling pthread_join()

Kevin
From a94d0cb7ff4202487ea980a029fa613c58d6d7c3 Mon Sep 17 00:00:00 2001
From: Kevin Wheatley <kevin.j.wheat...@gmail.com>
Date: Mon, 30 Apr 2018 16:33:51 +0100
Subject: [PATCH] The libvmaf filter tried to join on an invalid thread id

The thread id was invalid because it was not initialised
during the calls to init_complex_filtergraph.

This adds a flag to check for initialisation before trying to
peform the join.

Signed-off-by: Kevin Wheatley <kevin.j.wheat...@gmail.com>
---
 libavfilter/vf_libvmaf.c |9 -
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/libavfilter/vf_libvmaf.c b/libavfilter/vf_libvmaf.c
index 42c6b66..5d47a74 100644
--- a/libavfilter/vf_libvmaf.c
+++ b/libavfilter/vf_libvmaf.c
@@ -43,6 +43,7 @@ typedef struct LIBVMAFContext {
 int width;
 int height;
 double vmaf_score;
+int vmaf_thread_created;
 pthread_t vmaf_thread;
 pthread_mutex_t lock;
 pthread_cond_t cond;
@@ -228,6 +229,7 @@ static av_cold int init(AVFilterContext *ctx)
 s->gmain = av_frame_alloc();
 s->error = 0;
 
+s->vmaf_thread_created = 0;
 pthread_mutex_init(>lock, NULL);
 pthread_cond_init (>cond, NULL);
 
@@ -275,6 +277,7 @@ static int config_input_ref(AVFilterLink *inlink)
 av_log(ctx, AV_LOG_ERROR, "Thread creation failed.\n");
 return AVERROR(EINVAL);
 }
+s->vmaf_thread_created = 1;
 
 return 0;
 }
@@ -317,7 +320,11 @@ static av_cold void uninit(AVFilterContext *ctx)
 pthread_cond_signal(>cond);
 pthread_mutex_unlock(>lock);
 
-pthread_join(s->vmaf_thread, NULL);
+if (s->vmaf_thread_created)
+{
+pthread_join(s->vmaf_thread, NULL);
+s->vmaf_thread_created = 0;
+}
 
 av_frame_free(>gref);
 av_frame_free(>gmain);
-- 
1.7.1

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

Re: [FFmpeg-devel] [PATCH] libavfilter/vf_libvmaf.c The libvmaf filter tried to join on an invalid thread id

[Kevin Wheatley]

On Thu, May 3, 2018 at 12:38 PM, Ronald S. Bultje  wrote:
> Why?
>
> Your patch fixes a bug, I don't think we test bugs in fate, just features.

I am perhaps making an incorrect assumption that the code has a bug
because it is never tested and that by adding a test that simply tries
to use it, the vmaf code is more likely to work. I'm happy to not
write a test if that is preferred!

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

Re: [FFmpeg-devel] [PATCH] web/index: add news entry for release 4.1

[Kevin Wheatley]

On Wed, Nov 14, 2018 at 4:40 PM James Almer  wrote:
> +colorconstancy filter
> +AVS2 video decoder via libdavs2
[snip]

> +AVS2 video encoder via libxavs2

This line is in twice.

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

Re: [FFmpeg-devel] [PATCH] web/index: add news entry for release 4.1

[Kevin Wheatley]

OK, that makes sense I had wondered if it was a typo with d and x
being close to each other on the keyboard. and might be better written
as one line. Obviously not!

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

Re: [FFmpeg-devel] [RFC]lavc/mjpegenc_common: Fix aspect ratio

[Kevin Wheatley]

I came across a similar discussion here:

https://github.com/OpenImageIO/oiio/pull/1412
Kevin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC]lavc/mjpegenc_common: Fix aspect ratio

[Kevin Wheatley]

I guess it depends if you think that it is better to align with
defacto behaviour (and maybe clarify/correct the specifications) or
follow the specs and have users grumble about not matching the
behaviour of 'applications X, Y and Z', I'm pretty certain Photoshop
won't easily change its behaviour. The other messy option would be to
have a compatibility mode flag.

I don't think any of this is perfect!
Kevin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] colorspace: Rename jedec-p22 to ebu3213

[Kevin Wheatley]

On Fri, Aug 9, 2019 at 12:40 PM Hendrik Leppkes  wrote:
> The enum and our values are aligned to the H.273 / ISO/IEC 23001-8
> standards, which documents this entry to correspond to the primaries
> in use by vf_colorspace

That makes sense, although I'm now interested to find out where those
numbers came from as they didn't come out of the EBU document, guess
I'll have to ask next time I bump into somebody from the ITU or EBU,
the EBU numbers do come with allowable tolerances, but the blue
co-ordinate appears to fall outside of that.

Thanks

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

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

Re: [FFmpeg-devel] [PATCH] colorspace: Rename jedec-p22 to ebu3213

[Kevin Wheatley]

Is there a change to include the EBU primaries?

https://tech.ebu.ch/docs/tech/tech3213.pdf

White 0.313 0.329
Red 0.64 0.33
Green 0.29 0.60
Blue 0.15 0.06

as the ones currently called AVCOL_PRI_JEDEC_P22 are not those ones at
least in vf_colorspace.c

[AVCOL_PRI_JEDEC_P22] = { WP_D65, { 0.630, 0.340, 0.295, 0.605,
0.155, 0.077 } },

Kevin


On Fri, Aug 9, 2019 at 1:48 AM James Almer  wrote:
>
> On 8/8/2019 9:01 PM, rzu...@tebako.net wrote:
> > From: Raphaël Zumer 
> >
> > Internally, this adds an EBU3213 alias to JEDEC_P22,
> > and changes the name string to match ITU-T H.273.
> > ---
> >  libavcodec/options_table.h  | 2 +-
> >  libavfilter/vf_colorspace.c | 2 +-
> >  libavfilter/vf_setparams.c  | 2 +-
> >  libavfilter/vf_zscale.c | 2 +-
> >  libavutil/pixdesc.c | 2 +-
> >  libavutil/pixfmt.h  | 1 +
> >  6 files changed, 6 insertions(+), 5 deletions(-)
>
> This should be split into three patches. The first one adding the enum
> value to pixfmt.h, changing the string in pixdesc.c, bumping libavutil
> minor version, and adding an entry to doc/APIChanges. The subject should
> be something like "avutil/pixfmt: Add EBU Tech. 3213-E AVColorPrimaries
> value" plus the explanation that it's an alias for Jedec P22 (Or that
> one becoming an alias for the new one) after it.
> Then another patch changing the libavfilter option values and bumping
> libavfilter micro version, and another doing the same with libavcodec.
>
> >
> > diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> > index 4a266eca16..9d82188171 100644
> > --- a/libavcodec/options_table.h
> > +++ b/libavcodec/options_table.h
> > @@ -365,7 +365,7 @@ static const AVOption avcodec_options[] = {
> >  {"smpte428_1",  "SMPTE 428-1",0, AV_OPT_TYPE_CONST, {.i64 = 
> > AVCOL_PRI_SMPTE428 }, INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
> >  {"smpte431","SMPTE 431-2",0, AV_OPT_TYPE_CONST, {.i64 = 
> > AVCOL_PRI_SMPTE431 }, INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
> >  {"smpte432","SMPTE 422-1",0, AV_OPT_TYPE_CONST, {.i64 = 
> > AVCOL_PRI_SMPTE432 }, INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
> > -{"jedec-p22",   "JEDEC P22",  0, AV_OPT_TYPE_CONST, {.i64 = 
> > AVCOL_PRI_JEDEC_P22 },INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
> > +{"ebu3213", "EBU 3213-E", 0, AV_OPT_TYPE_CONST, {.i64 = 
> > AVCOL_PRI_JEDEC_P22 },INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
> >  {"unspecified", "Unspecified",0, AV_OPT_TYPE_CONST, {.i64 = 
> > AVCOL_PRI_UNSPECIFIED },  INT_MIN, INT_MAX, V|E|D, "color_primaries_type"},
> >  {"color_trc", "color transfer characteristics", OFFSET(color_trc), 
> > AV_OPT_TYPE_INT, {.i64 = AVCOL_TRC_UNSPECIFIED }, 1, INT_MAX, V|E|D, 
> > "color_trc_type"},
> >  {"bt709","BT.709",   0, AV_OPT_TYPE_CONST, {.i64 = 
> > AVCOL_TRC_BT709 },INT_MIN, INT_MAX, V|E|D, "color_trc_type"},
> > diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
> > index df6efffb3d..5f22f92507 100644
> > --- a/libavfilter/vf_colorspace.c
> > +++ b/libavfilter/vf_colorspace.c
> > @@ -968,7 +968,7 @@ static const AVOption colorspace_options[] = {
> >  ENUM("smpte431", AVCOL_PRI_SMPTE431,   "prm"),
> >  ENUM("smpte432", AVCOL_PRI_SMPTE432,   "prm"),
> >  ENUM("bt2020",   AVCOL_PRI_BT2020, "prm"),
> > -ENUM("jedec-p22",AVCOL_PRI_JEDEC_P22,  "prm"),
> > +ENUM("ebu3213",  AVCOL_PRI_JEDEC_P22,  "prm"),
> >
> >  { "trc","Output transfer characteristics",
> >OFFSET(user_trc),   AV_OPT_TYPE_INT, { .i64 = AVCOL_TRC_UNSPECIFIED 
> > },
> > diff --git a/libavfilter/vf_setparams.c b/libavfilter/vf_setparams.c
> > index fe298e5a06..80e61f851e 100644
> > --- a/libavfilter/vf_setparams.c
> > +++ b/libavfilter/vf_setparams.c
> > @@ -74,7 +74,7 @@ static const AVOption setparams_options[] = {
> >  {"smpte428",NULL,  0, AV_OPT_TYPE_CONST, 
> > {.i64=AVCOL_PRI_SMPTE428}, INT_MIN, INT_MAX, FLAGS, "color_primaries"},
> >  {"smpte431",NULL,  0, AV_OPT_TYPE_CONST, 
> > {.i64=AVCOL_PRI_SMPTE431}, INT_MIN, INT_MAX, FLAGS, "color_primaries"},
> >  {"smpte432",NULL,  0, AV_OPT_TYPE_CONST, 
> > {.i64=AVCOL_PRI_SMPTE432}, INT_MIN, INT_MAX, FLAGS, "color_primaries"},
> > -{"jedec-p22",   NULL,  0, AV_OPT_TYPE_CONST, 
> > {.i64=AVCOL_PRI_JEDEC_P22},INT_MIN, INT_MAX, FLAGS, "color_primaries"},
> > +{"ebu3213", NULL,  0, AV_OPT_TYPE_CONST, 
> > {.i64=AVCOL_PRI_JEDEC_P22},INT_MIN, INT_MAX, FLAGS, "color_primaries"},
> >
> >  {"color_trc", "select color transfer", OFFSET(color_trc), 
> > AV_OPT_TYPE_INT, {.i64=-1}, -1, AVCOL_TRC_NB-1, FLAGS, "color_trc"},
> >  {"auto", "keep the same color transfer",  0, AV_OPT_TYPE_CONST, 
> > {.i64=-1}, INT_MIN, INT_MAX, FLAGS, "color_trc"},
> > diff --git 

Re: [FFmpeg-devel] avfilter/vf_ciescope: add DCI-P3

[Kevin Wheatley]

Just a query, but I note that the gamma says 'GAMMA_REC709', this is
not the normal gamma I'd expect for a DCI P3 image, it would typically
be 2.6, so for correct translation into chromaticities, you would need
to use that to convert to linear RGB. It is also not the only option
for encoding the output image, although it appears it is the only
option listed in the structure.

What is that gamma parameter in the colour space structure supposed to
represent?

Thanks

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

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

[FFmpeg-devel] [PATCH] avformat/movenc: Support for variable timescale in mov containers

[Kevin Wheatley]

This pair of patches work together to facilitate user specified timescales for
the Movie Header Atom 'mvhd', which allows constant sample table durations for
all frame rates.

Currently there is a fixed timescale of 1000, which is not an even multiple of
all the typical video frame/field rates. This means when performing certain
duration based operations it is possible to be inaccurate.

The default behaviour is left at the current default defined by MOV_TIMESCALE,
but can be over ridden by using the -mov_timescale  option.

Typical values of 600 would work for 24, 25 and 30 FPS, for 23.976 and other
fractional rates could use 2997 same as Avid (or 24000 though caution
should be used when encoding long durations).


Example usage that has better behaviour than the current:

# Encode 50 frames at 24FPS and concatenate 5 copies
ffmpeg -f lavfi -i smptebars=duration=2.08:size=1920x1080:rate=24 \
-codec dnxhd -pix_fmt yuv422p -b:v 115M smptebars_dnx_1000.mov
cat < concat_1000.txt
file smptebars_dnx_1000.mov
file smptebars_dnx_1000.mov
file smptebars_dnx_1000.mov
file smptebars_dnx_1000.mov
file smptebars_dnx_1000.mov
EOF
ffmpeg -f concat -i concat_1000.txt -c copy smpte_concat_1000.mov
ffprobe smpte_concat_1000.mov

The output of ffprobe will show a frame rate of 23.99 due to the effect of the
sample durations in the stts entries.

With the new option of -mov_timescale set to 600:

ffmpeg -f lavfi -i smptebars=duration=2.08:size=1920x1080:rate=24 \
-codec dnxhd -pix_fmt yuv422p -b:v 115M -mov_timescale 600 smptebars_dnx_600.mov
cat < concat_600.txt
file smptebars_dnx_600.mov
file smptebars_dnx_600.mov
file smptebars_dnx_600.mov
file smptebars_dnx_600.mov
file smptebars_dnx_600.mov
EOF
ffmpeg -f concat -i concat_600.txt -c copy -mov_timescale 600
smpte_concat_600.mov
ffprobe smpte_concat_600.mov

The durations all line up, the stts table is smaller and no rounding
issues occur.


Kevin Wheatley (2):
  avformat/movenc: Add command line option to set base mov file
timescale
  avformat/movenc: Use base container timescale, instead of hard coded
default

 libavformat/movenc.c | 23 ---
 libavformat/movenc.h |  1 +
 2 files changed, 13 insertions(+), 11 deletions(-)

--
1.8.5.6


0002-avformat-movenc-Use-base-container-timescale-instead.patch
Description: Binary data


0001-avformat-movenc-Add-command-line-option-to-set-base-.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH v2 0/3] avformat/movenc: Support for variable timescale in mov containers

[Kevin Wheatley]

This set of patches work together to facilitate user specified timescales for
the Movie Header Atom 'mvhd', which allows constant sample table durations for
all frame rates.

Currently there is a fixed timescale of 1000, which is not an even multiple of
all the typical video frame/field rates. This means when performing certain
duration based operations it is possible to be inaccurate.

The default behaviour is left at the current default defined by MOV_TIMESCALE,
but can be over ridden by using the -mov_timescale  option.

Typical values of 600 would work for 24, 25 and 30 FPS, for 23.976 and other
fractional rates could use 2997 same as Avid (or 24000 though caution
should be used when encoding long durations).


Example usage that has better behaviour than the current:

# Encode 50 frames at 24FPS and concatenate 5 copies
ffmpeg -f lavfi -i smptebars=duration=2.08:size=1920x1080:rate=24 \
-codec dnxhd -pix_fmt yuv422p -b:v 115M smptebars_dnx_1000.mov
cat < concat_1000.txt
file smptebars_dnx_1000.mov
file smptebars_dnx_1000.mov
file smptebars_dnx_1000.mov
file smptebars_dnx_1000.mov
file smptebars_dnx_1000.mov
EOF
ffmpeg -f concat -i concat_1000.txt -c copy smpte_concat_1000.mov
ffprobe smpte_concat_1000.mov

The output of ffprobe will show a frame rate of 23.99 due to the effect of ther
sample durations in the stts entries.

With the new option of -mov_timescale set to 600:

ffmpeg -f lavfi -i smptebars=duration=2.08:size=1920x1080:rate=24 \
-codec dnxhd -pix_fmt yuv422p -b:v 115M -mov_timescale 600 smptebars_dnx_600.mov
cat < concat_600.txt
file smptebars_dnx_600.mov
file smptebars_dnx_600.mov
file smptebars_dnx_600.mov
file smptebars_dnx_600.mov
file smptebars_dnx_600.mov
EOF
ffmpeg -f concat -i concat_600.txt -c copy -mov_timescale 600
smpte_concat_600.mov
ffprobe smpte_concat_600.mov

The durations all line up, the stts table is smaller and no rounding
issues occur.

Kevin

Kevin Wheatley (3):
  avformat/movenc: Add command line option to set base mov file
timescale
  avformat/movenc: Use base container timescale, instead of hard coded
default
  avformat/movenc: Add an automatic timescale computation

 libavformat/movenc.c | 94 ++--
 libavformat/movenc.h |  2 ++
 2 files changed, 85 insertions(+), 11 deletions(-)


v2-0001-avformat-movenc-Add-command-line-option-to-set-ba.patch
Description: Binary data


v2-0002-avformat-movenc-Use-base-container-timescale-inst.patch
Description: Binary data


v2-0003-avformat-movenc-Add-an-automatic-timescale-comput.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] avformat/movenc: Support for variable timescale in mov containers

[Kevin Wheatley]

On Tue, Nov 5, 2019 at 4:52 PM Michael Niedermayer
 wrote:
> Assuming this doesnt violate any specifications and assuming that
> it works with all players.
> Then it would make sense to select this value automatically based
> on the stream timestamps or timebases.
> maybe there could be still a -mov_timescale but with an option for
> "auto" to autoselect a small one which allows accurate representation
> of most streams

Michael,

thanks for the feedback, I did consider this, but I am not that
familiar with the code base of FFmpeg to know what the 'right' data
fields to use are and at what point during the initialisation process
of the movenc code it is valid to use them.

There certainly seams to be a number of points at which the code tries
to compute a lowest common multiple in the general FFmpeg code and a
number of fields representing timebases, frame rates etc.

If somebody can help point me at what I should use, then I'm happy to
add something to automatically determine something.

In terms of the specification, the Apple documentation certainly
recommends the use of 600 on p221 of
https://developer.apple.com/standards/qtff-2001.pdf for most integer
frame rates, but as the following section in that document says this
does not work for 23.976 or 29.97.

For newer frame rates like 48 and any other number of 'new' content
types, those number fail so whilst  I'd suggest using the Apple number
for the well used rates, I guess a computation would be needed for
other things.

Finally what about the default behaviour, would it be the old use a
fixed setting or would we change the default to be automatic with the
option of explicitly passing in 1000 to mimic the previous behaviour
if required (plus any updates to fate to account for the change).

Thanks

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

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

Re: [FFmpeg-devel] [PATCH] avcodec/exr: add cineon lin2log trc

[Kevin Wheatley]

On Fri, Mar 6, 2020 at 10:19 AM Hendrik Leppkes  wrote:
> AVColorTransferCharacteristic should follow ISO/IEC 23001-8 and its
> following standards (ISO/IEC 23091 I believe). Not sure we have a
> solution for specialized variants, but adding one right there would
> collide with the next addition to the standard...

Agreed, A publicly "aligned twin text" version is available as ITU-T
H.273 https://www.itu.int/rec/T-REC-H.273-201612-I/en shows that value
is 'reserved'.

I would also suggest that there are a few implementations of the
Cineon log to lin conversion and as such I'd reject it as is.
You will need to pass in the parameters for the various variables
white, black etc. You will also need to add a parameter for the
density per code value constant (0.002 by default convention).
Not all implementations use a gain scaling, though strictly these are
no longer "Cineon" per se, but you will end up with needing various
camera vendor equivalents. This would result in a large proliferation
of equations, which I don't believe are core to the FFmpeg code.

You would perhaps be better creating a specialist filter and
implementing it using OCIO (as suggested
https://github.com/AcademySoftwareFoundation/tac/tree/master/gsoc), or
extending the current 3D LUT code to read Cinespace or other 3D LUT
formats that support a pre-shaper and handle the float->integer
conversion that way.

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

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

[FFmpeg-devel] [PATCH 3/3] avformat/movenc: Add an automatic timescale computation

[Kevin Wheatley]

Activated when the -mov_timescale command line/MOVMuxContext
parameter is set to 0 or less. If the user passes any value
greater than 0, then it will be used as-is. The default
value is kept unchanged at 1000.

When active, it uses the base assumption from the QuickTime
specification of 600 and combines the video stream time
bases to compute an accurate answer if possible.

In cases when the first stream result falls outside
MOV_MAX_AUTO_TIMESCALE or if a non-integer video stream is
encounted, then the first stream's time_base will be used as the
base.

Signed-off-by: Kevin Wheatley 
---
 libavformat/movenc.c | 71 
 libavformat/movenc.h |  1 +
 2 files changed, 72 insertions(+)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 508fa73..8edb848 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -6208,6 +6208,70 @@ static int 
mov_create_dvd_sub_decoder_specific_info(MOVTrack *track,
 return 0;
 }
 
+static int mov_determine_movie_timescale(AVFormatContext *s, MOVMuxContext 
*mov)
+{
+// Assume typical integer frame rates:
+// 600 is a common multiple of 24, 25, 30, 50, 60, etc.
+// see p221, https://developer.apple.com/standards/qtff-2001.pdf
+int timescale = 600;
+AVRational temp;
+int exact;
+int first_video_track = 1;
+
+// If the user specified a timescale, just use it as-is
+if (mov->mov_timescale > 0) {
+return mov->mov_timescale;
+}
+
+// Determine the timescale, based on the video stream time_base values
+for (int i = 0; i < s->nb_streams; i++) {
+AVStream *st = s->streams[i];
+if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
+
+// If the first video track is not an integer, use its denominator
+// as the basis of the remaining calculations
+if (first_video_track && st->time_base.num != 1) {
+timescale = st->time_base.den;
+av_log(s, AV_LOG_VERBOSE, "Using first video stream 
non-integer time_base: %d/%d\n",
+   st->time_base.den, st->time_base.num);
+}
+
+// determine if we can make an even multiple of the current 
timescale and the video track
+av_log(s, AV_LOG_VERBOSE, "Current estimated timescale: %d\n", 
timescale);
+av_log(s, AV_LOG_TRACE, "Stream #%d time_base: %d/%d\n", i,
+   st->time_base.num, st->time_base.den);
+
+// Try keep the time_scale within a sensible bound
+exact = av_reduce(, ,
+  (int64_t)timescale * st->time_base.num,
+  st->time_base.den,
+  MOV_MAX_AUTO_TIMESCALE);
+
+// Use a scaled version of the timescale
+if (exact) {
+timescale *= temp.den;
+av_log(s, AV_LOG_TRACE, "Adjusted timescale: %d/%d %d\n",
+   temp.den, temp.num, timescale);
+} else {
+// We overflowed try the denominator as is
+timescale = temp.den;
+av_log(s, AV_LOG_WARNING, "Timescale calculation out of bounds 
approximating %d/%d %d\n",
+   temp.den, temp.num, timescale);
+}
+
+if (first_video_track && ((timescale > MOV_MAX_AUTO_TIMESCALE) || 
!exact)) {
+timescale = st->time_base.den;
+av_log(s, AV_LOG_WARNING, "Potentially large timescale, "
+  "using first video stream time_base: 
%d/%d\n",
+   st->time_base.den, st->time_base.num);
+}
+first_video_track = 0;
+}
+}
+av_log(s, AV_LOG_VERBOSE, "Final timescale: %d\n", timescale);
+return timescale;
+}
+
 static int mov_init(AVFormatContext *s)
 {
 MOVMuxContext *mov = s->priv_data;
@@ -6266,6 +6330,13 @@ static int mov_init(AVFormatContext *s)
 mov->reserved_moov_size = -1;
 }
 
+mov->mov_timescale = mov_determine_movie_timescale(s, mov);
+if (mov->mov_timescale > MOV_MAX_AUTO_TIMESCALE) {
+av_log(s, AV_LOG_WARNING, "Potentially large timescale %d, use "
+  "-mov_timescale if you have issues\n",
+  mov->mov_timescale);
+}
+
 if (mov->use_editlist < 0) {
 mov->use_editlist = 1;
 if (mov->flags & FF_MOV_FLAG_FRAGMENT &&
diff --git a/libavformat/movenc.h b/libavformat/movenc.h
index 322968c..4d6b7b7 100644
--- a/libavformat/movenc.h
+++ b/libavformat/movenc.h
@@ -30,6 +30,7 @@
 #define MOV_FRAG_INFO_ALLOC_INCREMENT 64
 #define MOV_INDEX_CLUSTER_SIZE 1024
 #define MOV_TIMESCALE 1000
+#define MOV_MAX_

[FFmpeg-devel] [PATCH 2/3] avformat/movenc: Use base container timescale, instead of hard coded default

[Kevin Wheatley]

Signed-off-by: Kevin Wheatley 
---
 libavformat/movenc.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 7d79eca..508fa73 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2879,7 +2879,7 @@ static int mov_write_tkhd_tag(AVIOContext *pb, 
MOVMuxContext *mov,
   MOVTrack *track, AVStream *st)
 {
 int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
-  MOV_TIMESCALE, track->timescale,
+  mov->mov_timescale, track->timescale,
   AV_ROUND_UP);
 int version = duration < INT32_MAX ? 0 : 1;
 int flags   = MOV_TKHD_FLAG_IN_MOVIE;
@@ -3027,7 +3027,7 @@ static int mov_write_edts_tag(AVIOContext *pb, 
MOVMuxContext *mov,
   MOVTrack *track)
 {
 int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
-  MOV_TIMESCALE, track->timescale,
+  mov->mov_timescale, track->timescale,
   AV_ROUND_UP);
 int version = duration < INT32_MAX ? 0 : 1;
 int entry_size, entry_count, size;
@@ -3046,7 +3046,7 @@ static int mov_write_edts_tag(AVIOContext *pb, 
MOVMuxContext *mov,
 }
 }
 
-delay = av_rescale_rnd(start_dts + start_ct, MOV_TIMESCALE,
+delay = av_rescale_rnd(start_dts + start_ct, mov->mov_timescale,
track->timescale, AV_ROUND_DOWN);
 version |= delay < INT32_MAX ? 0 : 1;
 
@@ -3081,8 +3081,8 @@ static int mov_write_edts_tag(AVIOContext *pb, 
MOVMuxContext *mov,
 /* Avoid accidentally ending up with start_ct = -1 which has got a
  * special meaning. Normally start_ct should end up positive or zero
  * here, but use FFMIN in case dts is a small positive integer
- * rounded to 0 when represented in MOV_TIMESCALE units. */
-av_assert0(av_rescale_rnd(start_dts, MOV_TIMESCALE, track->timescale, 
AV_ROUND_DOWN) <= 0);
+ * rounded to 0 when represented in mov->mov_timescale units. */
+av_assert0(av_rescale_rnd(start_dts, mov->mov_timescale, 
track->timescale, AV_ROUND_DOWN) <= 0);
 start_ct  = -FFMIN(start_dts, 0);
 /* Note, this delay is calculated from the pts of the first sample,
  * ensuring that we don't reduce the duration for cases with
@@ -3316,7 +3316,7 @@ static int mov_write_mvhd_tag(AVIOContext *pb, 
MOVMuxContext *mov)
 if (mov->tracks[i].entry > 0 && mov->tracks[i].timescale) {
 int64_t max_track_len_temp = av_rescale_rnd(
 calc_pts_duration(mov, 
>tracks[i]),
-MOV_TIMESCALE,
+mov->mov_timescale,
 mov->tracks[i].timescale,
 AV_ROUND_UP);
 if (max_track_len < max_track_len_temp)
@@ -3345,7 +3345,7 @@ static int mov_write_mvhd_tag(AVIOContext *pb, 
MOVMuxContext *mov)
 avio_wb32(pb, mov->time); /* creation time */
 avio_wb32(pb, mov->time); /* modification time */
 }
-avio_wb32(pb, MOV_TIMESCALE);
+avio_wb32(pb, mov->mov_timescale);
 (version == 1) ? avio_wb64(pb, max_track_len) : avio_wb32(pb, 
max_track_len); /* duration of longest track */
 
 avio_wb32(pb, 0x0001); /* reserved (preferred rate) 1.0 = normal */
@@ -5921,7 +5921,7 @@ static int mov_create_chapter_track(AVFormatContext *s, 
int tracknum)
 
 track->mode = mov->mode;
 track->tag = MKTAG('t','e','x','t');
-track->timescale = MOV_TIMESCALE;
+track->timescale = mov->mov_timescale;
 track->par = avcodec_parameters_alloc();
 if (!track->par)
 return AVERROR(ENOMEM);
@@ -5982,8 +5982,8 @@ static int mov_create_chapter_track(AVFormatContext *s, 
int tracknum)
 AVChapter *c = s->chapters[i];
 AVDictionaryEntry *t;
 
-int64_t end = av_rescale_q(c->end, c->time_base, 
(AVRational){1,MOV_TIMESCALE});
-pkt.pts = pkt.dts = av_rescale_q(c->start, c->time_base, 
(AVRational){1,MOV_TIMESCALE});
+int64_t end = av_rescale_q(c->end, c->time_base, 
(AVRational){1,mov->mov_timescale});
+pkt.pts = pkt.dts = av_rescale_q(c->start, c->time_base, 
(AVRational){1,mov->mov_timescale});
 pkt.duration = end - pkt.dts;
 
 if ((t = av_dict_get(c->metadata, "title", NULL, 0))) {
@@ -6518,7 +6518,7 @@ static int mov_init(AVFormatContext *s)
 } else if (st->codecpar->codec_type == AVMEDIA_TYPE_DATA) {
 track->timescale = st->time_base.den;
 } e

[FFmpeg-devel] [PATCH v3 0/3] avformat/movenc: Support for variable timescale in mov containers

[Kevin Wheatley]

v3 - rebased to current master

This set of patches work together to facilitate user specified timescales for
the Movie Header Atom 'mvhd', which allows constant sample table durations for
all frame rates.

Currently there is a fixed timescale of 1000, which is not an even multiple of
all the typical video frame/field rates. This means when performing certain
duration based operations it is possible to be inaccurate.

The default behaviour is left at the current default defined by MOV_TIMESCALE,
but can be over ridden by using the -mov_timescale  option.

Typical values of 600 would work for 24, 25 and 30 FPS, for 23.976 and other
fractional rates could use 2997 same as Avid (or 24000 though caution
should be used when encoding long durations).

An automatic mode has been added if a negative value is passed to the option.
This will compute an appropriate time scale starting from the lowest numbered
video stream's time scale.


Example usage that has better behaviour than the current:

# Encode 50 frames at 24FPS and concatenate 5 copies
ffmpeg -f lavfi -i smptebars=duration=2.08:size=1920x1080:rate=24 \
-codec dnxhd -pix_fmt yuv422p -b:v 115M smptebars_dnx_1000.mov
cat < concat_1000.txt
file smptebars_dnx_1000.mov
file smptebars_dnx_1000.mov
file smptebars_dnx_1000.mov
file smptebars_dnx_1000.mov
file smptebars_dnx_1000.mov
EOF
ffmpeg -f concat -i concat_1000.txt -c copy smpte_concat_1000.mov
ffprobe smpte_concat_1000.mov

The output of ffprobe will show a frame rate of 23.99 due to the effect of ther
sample durations in the stts entries.

With the new option of -mov_timescale set to 600:

ffmpeg -f lavfi -i smptebars=duration=2.08:size=1920x1080:rate=24 \
-codec dnxhd -pix_fmt yuv422p -b:v 115M -mov_timescale 600 smptebars_dnx_600.mov
cat < concat_600.txt
file smptebars_dnx_600.mov
file smptebars_dnx_600.mov
file smptebars_dnx_600.mov
file smptebars_dnx_600.mov
file smptebars_dnx_600.mov
EOF
ffmpeg -f concat -i concat_600.txt -c copy -mov_timescale 600 
smpte_concat_600.mov
ffprobe smpte_concat_600.mov

The durations all line up, the stts table is smaller and no rounding issues 
occur.

Kevin

Kevin Wheatley (3):
  avformat/movenc: Add command line option to set base mov file
timescale
  avformat/movenc: Use base container timescale, instead of hard coded
default
  avformat/movenc: Add an automatic timescale computation

 libavformat/movenc.c | 94 ++--
 libavformat/movenc.h |  2 ++
 2 files changed, 85 insertions(+), 11 deletions(-)

-- 
1.8.5.6

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

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

[FFmpeg-devel] [PATCH 1/3] avformat/movenc: Add command line option to set base mov file timescale

[Kevin Wheatley]

Signed-off-by: Kevin Wheatley 
---
 libavformat/movenc.c | 1 +
 libavformat/movenc.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 253cff8..7d79eca 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -91,6 +91,7 @@ static const AVOption options[] = {
 { "min_frag_duration", "Minimum fragment duration", 
offsetof(MOVMuxContext, min_fragment_duration), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 
INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
 { "frag_size", "Maximum fragment size", offsetof(MOVMuxContext, 
max_fragment_size), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, 
AV_OPT_FLAG_ENCODING_PARAM},
 { "ism_lookahead", "Number of lookahead entries for ISM files", 
offsetof(MOVMuxContext, ism_lookahead), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 
INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
+{ "mov_timescale", "set timescale of mov container", 
offsetof(MOVMuxContext, mov_timescale), AV_OPT_TYPE_INT, {.i64 = 
MOV_TIMESCALE}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
 { "video_track_timescale", "set timescale of all video tracks", 
offsetof(MOVMuxContext, video_track_timescale), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 
INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
 { "brand","Override major brand", offsetof(MOVMuxContext, 
major_brand),   AV_OPT_TYPE_STRING, {.str = NULL}, .flags = 
AV_OPT_FLAG_ENCODING_PARAM },
 { "use_editlist", "use edit list", offsetof(MOVMuxContext, use_editlist), 
AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, AV_OPT_FLAG_ENCODING_PARAM},
diff --git a/libavformat/movenc.h b/libavformat/movenc.h
index 997b2d6..322968c 100644
--- a/libavformat/movenc.h
+++ b/libavformat/movenc.h
@@ -205,6 +205,7 @@ typedef struct MOVMuxContext {
 AVIOContext *mdat_buf;
 int first_trun;
 
+int mov_timescale;
 int video_track_timescale;
 
 int reserved_moov_size; ///< 0 for disabled, -1 for automatic, size 
otherwise
-- 
1.8.5.6

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

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

Re: [FFmpeg-devel] [PATCH] avfilter/vf_scale: set RGB to always be full range

[Kevin Wheatley]

On Wed, Sep 16, 2020 at 9:43 PM Jan Ekström  wrote:
>
> On Wed, Sep 16, 2020 at 11:38 PM Michael Niedermayer
>  wrote:
> > Does anything document the meaning of range or AVCOL_RANGE for RGB ?
> > The enum is documented as "MPEG vs JPEG YUV range."
> >
> > What is limited range RGB ? what would the ranges of R,G,B be and where
> > is that specified ?

EBU only describes full range for 'file' based images

https://tech.ebu.ch/docs/r/r103.pdf

but ITU 
https://www.itu.int/dms_pubrec/itu-r/rec/bt/R-REC-BT.2100-2-201807-I!!PDF-E.pdf

does describe narrow range RGB on p9

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

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

Re: [FFmpeg-devel] [PATCH] libavutil/cpu: Fix definition of _GNU_SOURCE so it occurs before other includes

[Kevin Wheatley]

On Mon, Apr 12, 2021 at 12:22 PM  wrote:
>
> From: Kevin Wheatley 
>
> This fix moves the potential definition of _GNU_SOURCE prior to
> any includes of system header files as required by the documentation
> https://www.gnu.org/software/libc/manual/html_node/Feature-Test-Macros.html
>
> This corrects the CPU_COUNT macro availability, resulting in
> sched_getaffinity() being called on Linux systems. This then correctly
> returns the number of CPUs when run under containers and other cases
> where processor affinity has been setup prior to running FFmpeg
>

bump

As an FYI the issue is triggered because the inclusion of the other
system headers prior to the _GNU_SOURCE definition results in the
CPU_COUNT macro not being defined, and so the affinity based CPU
counting fails to be used and so the cpu count is incorrectly
returned.

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

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

Re: [FFmpeg-devel] [PATCH] movenc: add movie_timescale option instead of hardcoding 1000

[Kevin Wheatley]

This is quite similar to my patch suggestion from last year:

http://ffmpeg.org/pipermail/ffmpeg-devel/2020-April/261088.html

mine was missing some docs updates, and I followed the existing naming
scheme of prefixing the argument as 'mov_' rather than movie, but
otherwise is quite similar

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

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

Re: [FFmpeg-devel] [PATCH 2/2] Require compilers to support C17.

[Kevin Wheatley]

On Fri, Feb 9, 2024 at 11:22 AM Dominik 'Rathann' Mierzejewski
 wrote:
> Even so, a C17-supporting compiler (gcc 11.2.1) is available for CentOS 7
> in the devtoolset-11-gcc package (from
> http://mirror.centos.org/centos/7/sclo/x86_64/rh/).

As a 'User' of the FFmpeg project, we have a lot of CentOS/RHEL 7/etc
based headless machines which run FFmpeg and thus we compile versions
of FFmpeg on this platform. This is currently quite common across the
VFX industry as most of the industry is still in the process of moving
away from this to something more recent see https://vfxplatform.com/.

The availability of  "newer" compilers via the devtool set should make
 this proposed requirement a relatively small obstacle for building
newer versions. As such I think that communicating the proposed change
and its implications in the next release versions and the FFmpeg
website should be sufficient, if similar notes for other common
platforms could be added to the release notes/changelog then at least
for users of these Fedora/RHEL derivatives should be fine.

I'm assuming anybody able to compile a custom FFmpeg build, is also
able to arrange to install a prebuilt compiler, so as long as the
dependency doesn't bleed through to a runtime one, all should be good.

On the topic of why users may be so far behind the "current" for the
OS is down to several factors, in VFX  and animation it is not
uncommon to work upon a project for many years prior to releasing,
this means we tend to lock down software versions in use for long
periods of time (switching out compilers, libraries etc can all change
the results of computation and thus our images). Running multiple
projects at once can make changing the OS a long duration process.

We've recently seen a similar issue with vscode bumping glibc
dependencies https://github.com/microsoft/vscode/issues/203375 though
switching glibc versions is a lot more awkward than a compiler
requirement.

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

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