Re: [FFmpeg-devel] Gaining access to a encoder context in avformat?
On Tue, Feb 24, 2015 at 10:10:21AM +, Kevin Wheatley wrote:
+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;
+}
Note for future patches:
If you have a case that returns, please put it in the if part.
That way you do not need the else part and in this specific
case you would have avoided two nesting levels, IMHO making the
code a lot more readable.
___
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?
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] Gaining access to a encoder context in avformat?
On Tue, Feb 24, 2015 at 10:10:21AM +, Kevin Wheatley wrote: 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. i think that patch is a big improvment relative to what is in git so applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Democracy is the form of government in which you can choose your dictator signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Gaining access to a encoder context in avformat?
On Fri, Feb 20, 2015 at 07:57:05AM +, Kevin Wheatley wrote: Sent on the go... On 19 Feb 2015, at 18:35, Michael Niedermayer michae...@gmx.at wrote: theres no encoder, the data could instead come from another mov file or a binary encoder used by some user application with libavformat Michael, Thanks for your response, I must be confused then. What I thought was based on the following. I have say a set of image files that I want to encode as dnxhd in QuickTime, so ffmpeg sets up the encoder, and during each image's encoding the encoder needs some information such as if the movie is encoded with interlaced fields, and also the specifics of the resolution and bit rates there's a codec/compression Id associated with those settings. When writing the avid atoms, they also should have those same values encoded in them. This is important as if instead of starting from image files we start from an existing movie with the atom present, we would not want to use values based on that atom, which is what s currently done, I wanted to find the dnxencoder structure to correctly populate the atom. 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 ... [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many things microsoft did are stupid, but not doing something just because microsoft did it is even more stupid. If everything ms did were stupid they would be bankrupt already. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Gaining access to a encoder context in avformat?
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?
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?
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. signature.asc Description: Digital signature ___ 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?
Hi, On Fri, Feb 20, 2015 at 6:56 AM, Kevin Wheatley kevin.j.wheat...@gmail.com wrote: 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 Why wouldn't the mov demuxer parse these atoms and fill them in the same way the dnxhd encoder does? The idea is that anything that outputs encoded dnxhd frames should fill these data structures (where-ever they are, but should probably be in some lavformat structure, not a lavcodec structure), and for backwards compat you should also deal with the fact that they might not be there. Ronald ___ 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?
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] Gaining access to a encoder context in avformat?
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 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 ? 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? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are too smart to engage in politics are punished by being governed by those who are dumber. -- Plato signature.asc Description: Digital signature ___ 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?
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
Re: [FFmpeg-devel] Gaining access to a encoder context in avformat?
On Fri, Feb 20, 2015 at 02:48:31PM +, Kevin Wheatley wrote: 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. 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 GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
